YQ-2803 Exception checking in checkpoint storages #1540
Conversation
|
⚪
|
|
⚪
|
|
⚪
|
|
⚪
|
ydb/core/fq/libs/ydb/ydb.cpp
Outdated
| return StatusToIssues(future.GetValue()); | ||
| } catch (...) { | ||
| TIssues issues; | ||
| issues.AddIssue(CurrentExceptionMessage()); |
There was a problem hiding this comment.
хорошо бы разделять ошибки внутри нашей системы от ошибок внутри базы. то есть текст сообщения тут лучше все же придумать и к нему уже прибавить текст исключения
There was a problem hiding this comment.
Added text "StatusToIssues failed"
| return TCreateCheckpointResult(TString(), std::move(issues)); | ||
| } else { | ||
| return TCreateCheckpointResult(checkpointContext->CheckpointGraphDescriptionContext->GraphDescId, NYql::TIssues()); | ||
| try { |
There was a problem hiding this comment.
почему бы не использовать в этих местах функцию StatusToIssues ?
| return TCreateCheckpointResult(checkpointContext->CheckpointGraphDescriptionContext->GraphDescId, NYql::TIssues()); | ||
| try { | ||
| if (NYql::TIssues issues = StatusToIssues(future.GetValue())) { | ||
| return TCreateCheckpointResult(TString(), std::move(issues)); |
There was a problem hiding this comment.
if/else тут не нужны
descId = ...;
issues = StatusToIssues(...);
return TCreateCheckpointResult(descId, std::move(issues));
|
⚪
|
|
⚪
|
|
|
||
| return StatusToIssues(future).Apply( | ||
| [result] (const TFuture<TIssues>& future) { | ||
| return std::make_pair(std::move(result->Size), std::move(future.GetValue())); |
There was a problem hiding this comment.
везде вместо GetValue предлагают использовать ExtractValue, чтобы делался мув и не было копий
There was a problem hiding this comment.
У Apply() сигнатура должна быть ...(const TFuture&>()) (future.h), а ExtractValue не констатная. Поэтому так не получается
There was a problem hiding this comment.
тогда давай убирать бесполезные std::move. в этой строке их две штуки, как я понимаю
There was a problem hiding this comment.
Убрал лишние std::move
|
|
||
| return StatusToIssues(future).Apply( | ||
| [result] (const TFuture<TIssues>& future) { | ||
| return std::make_pair(std::move(result->Size), std::move(future.GetValue())); |
There was a problem hiding this comment.
тогда давай убирать бесполезные std::move. в этой строке их две штуки, как я понимаю
ydb/core/fq/libs/ydb/ut/ydb_ut.cpp
Outdated
| UNIT_ASSERT(issues.Size() == 1); | ||
| UNIT_ASSERT(issues.ToString().Contains(text)); | ||
| } | ||
| }; |
|
⚪
|
|
⚪
|
|
⚪
|
|
⚪
|
* Exception check * Add new line * remove comment * Use StatusToIssues() * remove some std::move * remove ;
Changelog entry
Changelog category
Additional information