YQ-4534 Fix describe federated logbroker #26327
Conversation
|
🟢 |
| } | ||
|
|
||
| NThreading::TFuture<TGetSchemeEntryResult> GetSchemeEntryType( | ||
| NThreading::TFuture<TGetSchemeEntryResult> GetSchemeEntryTypeImpl( |
There was a problem hiding this comment.
Предлагаю перенести реализацию в анонимный namespace (так как она больше не определена в .h файле)
| opts | ||
| .DiscoveryEndpoint(endpoint) | ||
| .Database(database) | ||
| .Database(!addRoot ? database : "/Root" + database) |
There was a problem hiding this comment.
Может быть тут заменить !addRoot -> addRoot? (и переставить местами значения)
|
|
||
| return schemeClient->DescribePath(path) | ||
| .Apply([actorSystem = NActors::TActivationContext::ActorSystem(), p = path, sc = schemeClient, database, endpoint](const NThreading::TFuture<NYdb::NScheme::TDescribePathResult>& result) { | ||
| return schemeClient->DescribePath(!addRoot ? path : "/Root" + path) |
There was a problem hiding this comment.
Может быть тут заменить !addRoot -> addRoot? (и переставить местами значения)
| bool useTls, | ||
| const TString& structuredTokenJson, | ||
| const TString& path) { | ||
| return GetSchemeEntryTypeImpl(NActors::TActivationContext::ActorSystem(), federatedQuerySetup, endpoint, database, useTls, structuredTokenJson, path, false); |
There was a problem hiding this comment.
Предлагаю вызвать CanonizePath от database перед дописывание /Root, чтобы гарантировать наличие '/' в начале:
| return GetSchemeEntryTypeImpl(NActors::TActivationContext::ActorSystem(), federatedQuerySetup, endpoint, database, useTls, structuredTokenJson, path, false); | |
| return GetSchemeEntryTypeImpl(NActors::TActivationContext::ActorSystem(), federatedQuerySetup, endpoint, CanonizePath(database), useTls, structuredTokenJson, path, false); |
|
|
||
| NThreading::TFuture<TGetSchemeEntryResult> GetSchemeEntryType( | ||
| NThreading::TFuture<TGetSchemeEntryResult> GetSchemeEntryTypeImpl( | ||
| TActorSystem *actorSystem, |
|
⚪ ⚪ Ya make output | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
yumkam
left a comment
yumkam
left a comment
There was a problem hiding this comment.
(посравнивал код - если получили CLIENT_UNAUTH, ретраимся с добавлением /Root в начало database, других отличий нет, lgtm)
| auto describePathResult = result.GetValue(); | ||
| TGetSchemeEntryResult res; | ||
| if (!describePathResult.IsSuccess()) { | ||
| if (describePathResult.GetStatus() == NYdb::EStatus::CLIENT_UNAUTHENTICATED && !addRoot) { |
There was a problem hiding this comment.
А имеет ли смысл когда-либо делать /Root/Root? (соответственно, можно смотреть на .starts_with("/Root")); впрочем, как сейчас тоже нормально
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ ⚪ Ya make output | Test bloat | Test bloat
⚪ Ya make output | Test bloat | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ ⚪ Ya make output | Test bloat | Test bloat
⚪ Ya make output | Test bloat | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ ⚪ Ya make output | Test bloat | Test bloat
⚪ Ya make output | Test bloat | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
5 participants
Changelog entry
...
Changelog category
Description for reviewers
...