-
Notifications
You must be signed in to change notification settings - Fork 6.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implementing reconfig
, sync
, exists
commands for keeper-client
#54201
Conversation
This is an automated comment for commit 4f5d132 with description of existing statuses. It's updated for the latest CI running
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but we really need tests
something like we did for the client in tests/integration/helpers/client.py
we should be able to do with keeper-client
if (!processQueryText(input)) | ||
break; | ||
|
||
std::cout << "\a\a\a\a" << std::endl; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a hack that hopefully will be removed after supporting JSON output format.
reconfig
command for keeper-clientreconfig
, sync
, exists
commands for keeper-client
pass | ||
|
||
|
||
class KeeperClient(object): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️
programs/keeper-client/Commands.cpp
Outdated
if (operation == static_cast<Int64>(ReconfigCommand::Operation::ADD)) | ||
joining = query->args[1].safeGet<DB::String>(); | ||
else if (operation == static_cast<Int64>(ReconfigCommand::Operation::REMOVE)) | ||
leaving = query->args[1].safeGet<DB::String>(); | ||
else if (operation == static_cast<Int64>(ReconfigCommand::Operation::SET)) | ||
new_members = query->args[1].safeGet<DB::String>(); | ||
else | ||
UNREACHABLE(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
switch statements?
zk1 = get_fake_zk(node1) | ||
config, _ = zk1.get("/keeper/config") | ||
config = config.decode("utf-8") | ||
global zk1, zk2, zk3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new client in tests seems to be flaky, investigating it. |
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Implementation of
reconfig
(#49450),sync
, andexists
commands for keeper-client.#54129