Skip to content
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

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

mariadb-AlanMologorsky
Copy link
Contributor

  • 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
@mariadb-AlanMologorsky mariadb-AlanMologorsky changed the title MCOL-5638 Improve Client Error Messaging and async broadcasting config. [DNM] MCOL-5638 Improve Client Error Messaging and async broadcasting config. Feb 7, 2025
if error_messages:
errors_str = ', '.join(error_messages)
final_message = (
f'Broadcasting config to nodes failed with errors: {errors_str}'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice!

Copy link
Collaborator

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.

Copy link
Collaborator

@drrtuy drrtuy left a 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.

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.

2 participants