-
Notifications
You must be signed in to change notification settings - Fork 67
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
[DNM] MCOL-5638 Improve Client Error Messaging and async broadcasting config. #3400
base: develop
Are you sure you want to change the base?
[DNM] MCOL-5638 Improve Client Error Messaging and async broadcasting config. #3400
Conversation
mariadb-AlanMologorsky
commented
Feb 7, 2025
- fix(Cmapi): broadcast_new_config function using aiohttp and improved errors handling mechanics
- fix(Cmapi): requirements updating aiohttp version and it's dependencies
- fix(Cmapi, deps): unused imports
[add] cluster api client class [fix] cli cluster_app using cluster api client [add] ClusterAction Enum [add] toggle_cluster_state function to reduce code duplication
[fix] client class methods [fix] Transaction management at endpoint handling level [fix] add_node and set_mode methods to use TransactionManager
MAJOR: Some logic inside node remove changed significantly using active nodes list from Columnstore.xml to broadcast config after remove. [fix] remove node logic to use Transaction manager [fix] cluster set api key call using totp on a top level cli call [add] missed docstrings
…asting config. * fix(Cmapi): broadcast_new_config function using aiohttp and improved errors handling mechanics * fix(Cmapi): requirements updating aiohttp version and it's dependencies * fix(Cmapi, deps): unused imports
if error_messages: | ||
errors_str = ', '.join(error_messages) | ||
final_message = ( | ||
f'Broadcasting config to nodes failed with errors: {errors_str}' |
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.
Nice!
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.
That would be helpful to have nodes ids here together with the corresponding errors.
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.
Plz add nodes ids to the error msg.