Test: Check settings validation #16733
Conversation
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
⚪ Test history | 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 |
|
🟢 |
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
⚪ Test history | 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 |
| auto grpcPort = cluster.MutableSettings()->Add(); | ||
| *grpcPort->MutableName() = "grpcPort"; | ||
| *grpcPort->MutableValue() = value; | ||
| if (auto value = properties.Value("http_endpoint", ""); !value.empty()) { |
There was a problem hiding this comment.
Можно наверное упростить и написать:
if (auto value = properties.Value("http_endpoint", "")) {
(аналогично с ифом ниже)
| if (attr.name() == "http_endpoint"sv) { | ||
| source.SetHttpEndpoint(attr.value()); | ||
| } | ||
| else if (attr.name() == "grpc_endpoint"sv) { |
There was a problem hiding this comment.
Обычно в коде рядом else идёт на той же строчке, что и }:
} else if (attr.name() == "grpc_endpoint"sv) {
Аналогично с else ниже
| } | ||
|
|
||
| if (source.GetHttpEndpoint().empty() || source.GetGrpcEndpoint().empty()) { | ||
| throw yexception() << "Specify both http_endpoint and grpc_endpoint for solomon source."; |
There was a problem hiding this comment.
Не могу найти по коду валидации того что http_endpoint и grpc_endpoint заполнены на более ранних оптимизациях. Она должна быть сделана дополнительно на более раннем этапе, так как ошибки возникающие в dq integration приводят к завершению запроса с INTERNAL ERROR (можно добавить валидацию на load metadata / type annotation)
| @@ -17,12 +17,13 @@ def parse_args(): | |||
| parser.add_argument("--auth", type=str, required=False, help="Allowed value for Authorization header") | |||
| parser.add_argument("--shard", type=str, required=False, action='append', | |||
| help="Allowed shard id in form $project_name/$service_name/$cluster_name") | |||
| parser.add_argument("--port", type=int, required=False, default=31000, help="Listen HTTP port") | |||
| parser.add_argument("--http_port", type=int, required=False, default=31000, help="Listen HTTP port") | |||
| parser.add_argument("--grpc_port", type=int, required=False, default=32000, help="Listen GRPC port") | |||
There was a problem hiding this comment.
Немного не обычно выглядит snake case в аргументах, может быть переименовать в --http-port и --grpc-port? Но я не уверен как лучше
| result["from"] = str(request.from_time) | ||
| result["to"] = str(request.to_time) | ||
| result["program"] = f"program length {len(str(request.queries[0].value))}" | ||
| if (request.downsampling.HasField("disabled")): |
There was a problem hiding this comment.
Кажется скобки для python в if излишни, if request.downsampling.HasField("disabled"): должно работать
| logger.debug('ReadRequest: %s', request) | ||
|
|
||
| project = request.container.project_id | ||
| if (project == "invalid"): |
There was a problem hiding this comment.
Тоже можно наверное опустить скобки
| @@ -0,0 +1,3 @@ | |||
| RECURSE( | |||
| reading | |||
| ) | |||
There was a problem hiding this comment.
Не хватает перевода строки
| ydb/tests/library/test_meta | ||
| ) | ||
|
|
||
| END() |
There was a problem hiding this comment.
Не хватает перевода строки
| error = generic_error | ||
|
|
||
| assert (error is not None) | ||
| assert (error_msg in extract_issue_messages(error)) |
There was a problem hiding this comment.
В assert на сколько я знаю тоже скобки можно убрать
Changelog entry
...
Changelog category
Description for reviewers
new tests (and test infra) for solomon/reading test suit (#16383)