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

feat: disable kbcli addons in batch #313

Conversation

SuperAayush
Copy link
Contributor

The PR contains the code to make the kbcli to disable more than one addon at once.

Fixes: #237

@github-actions github-actions bot added the size/M Denotes a PR that changes 30-99 lines. label Apr 10, 2024
Copy link
Contributor

Choose a reason for hiding this comment

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

do not commit changes of submodules.

@1aal
Copy link
Contributor

1aal commented Apr 10, 2024

Hi @SuperAayush , nice to receive your PR!
I personally have some thoughts about your PR, I think there are some object that we don't need to repeat the initialization such as dynamic.Interface in addonCmdOpts.
Another question is what happens if, in the process of disabling a bunch of addons, one of them fails to be disabled. Like
kbcli addon disable apecloud-mysql WRONG redis, we expect the apecloud-mysql and redis to successfully disable and for theWRONG , it will report an error. Or neither of them will be disabled, with the corresponding error message

@SuperAayush
Copy link
Contributor Author

Hey @1aal, I agree with your suggestions I had a chat regarding the same with @shanshanying over a call.
While adding the test case I ran into multiple errors and debugged them as well, since there is no working test case for the enable command I can't debug the complete process.
The code block that would trigger the test is commented:

// When("Enable an addon", func() {

Also after validating the test, the process is stuck here:

return fmt.Errorf("addon %s INSTALLABLE-SELECTOR has no matching requirement", o.Names)

I have even tried to fetch the selector labels and they match exactly but the process is still stuck.
Happy to connect with you over a call to debug this.

cc - @shanshanying

Thanks!!

Copy link

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment

@github-actions github-actions bot added the Stale label May 27, 2024
@nayutah
Copy link
Collaborator

nayutah commented Jul 25, 2024

@SuperAayush this pr has been refined and merged to main branch a97d12d

@nayutah nayutah closed this Jul 25, 2024
@github-actions github-actions bot added this to the Release 0.9.0 milestone Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/M Denotes a PR that changes 30-99 lines. Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Improvement] kbcli disable addons in batch
5 participants