Skip to content

Conversation

@nisqatsi
Copy link
Member

Now blockstore-client silently ignores unknown params in nbs-client.txt config file.

@nisqatsi nisqatsi requested review from agalibin and drbasic November 28, 2025 09:09
@github-actions
Copy link
Contributor

github-actions bot commented Nov 28, 2025

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit c108692.

TESTS PASSED ERRORS FAILED FAILED BUILD SKIPPED MUTED?
9636 9634 0 1 0 1 0

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit c108692.

TESTS PASSED ERRORS FAILED FAILED BUILD SKIPPED MUTED?
45 45 0 0 0 0 0

agalibin
agalibin previously approved these changes Nov 28, 2025
@nisqatsi nisqatsi added the recheck Add this label to relaunch checks, it will be automatically removed label Dec 4, 2025
@github-actions github-actions bot removed the recheck Add this label to relaunch checks, it will be automatically removed label Dec 4, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Dec 4, 2025

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit c108692.

TESTS PASSED ERRORS FAILED FAILED BUILD SKIPPED MUTED?
9636 9634 0 1 0 1 0

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit c108692.

TESTS PASSED ERRORS FAILED FAILED BUILD SKIPPED MUTED?
45 45 0 0 0 0 0

@nisqatsi nisqatsi removed the request for review from drbasic December 5, 2025 09:12
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Лишний файл?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Точно

NProto::TClientAppConfig appConfig;
if (NFs::Exists(ConfigFile)) {
ParseFromTextFormat(ConfigFile, appConfig);
ParseFromTextFormat(ConfigFile, appConfig, EParseFromTextFormatOption::AllowUnknownField);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Completely ignoring unknown fields is a bit dangerous, let's at least log them - you can do it easily via the third ParseFromTextFormat() parameter - warningStream

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

simple Cerr would suffice

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@nisqatsi nisqatsi force-pushed the users/usenkoav/NBS_6385-blockstore_client-ignore_unknown_conf_params branch from c108692 to 1f4f506 Compare December 5, 2025 12:33
@github-actions
Copy link
Contributor

github-actions bot commented Dec 5, 2025

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 1f4f506.

TESTS PASSED ERRORS FAILED FAILED BUILD SKIPPED MUTED?
9636 9633 0 2 0 1 0

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit 1f4f506.

TESTS PASSED ERRORS FAILED FAILED BUILD SKIPPED MUTED?
47 47 0 0 0 0 0

NProto::TClientAppConfig appConfig;
if (NFs::Exists(ConfigFile)) {
ParseFromTextFormat(ConfigFile, appConfig);
ParseFromTextFormat(ConfigFile, appConfig, EParseFromTextFormatOption::AllowUnknownField, &GetOutputStream());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GetOutputStream() -> GetErrorStream()

if (NFs::Exists(ConfigFile)) {
ParseFromTextFormat(ConfigFile, appConfig);
ParseFromTextFormat(ConfigFile, appConfig, EParseFromTextFormatOption::AllowUnknownField, &GetOutputStream());
GetOutputStream() << Endl;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if ParseFromTextFormat doesn't print a \n to this stream, then it's better to use TStringStream ss and then do something like

if (ss.Str().Size()) {
    GetErrorStream() << ss.Str() << '\n';
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants