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

[CLI-2908] Add plugin uninstall command #2761

Merged
merged 9 commits into from
May 29, 2024
Merged

[CLI-2908] Add plugin uninstall command #2761

merged 9 commits into from
May 29, 2024

Conversation

tmalikconfluent
Copy link
Contributor

@tmalikconfluent tmalikconfluent commented May 13, 2024

Release Notes

Breaking Changes

  • The "name" field in in the confluent plugin search has been renamed to "id"; a new "name" field has been added in its place

New Features

  • Added confluent plugin uninstall command for Confluent Platform

Checklist

  • Leave this box unchecked if features are not yet available in production

What

Added plugin uninstall command to remove the plugins that are no longer needed for a user. Also added the ID for a particular plugin in the command plugin list (which can be used as the flag in uninstall) and changed the one of the column names in plugin search for the sake of consistency with list command

References

https://confluentinc.atlassian.net/browse/CLI-2908

Test & Review

Created a new function in the TestPluginUninstall in the plugin_test.go class to test the unistall functionality added. This involved creating a dummy plugin, uninstalling it and checking that the plugin list command gives the expected output after the uninstall.
For manual testing, added one of the plugins using the install. Verified it using plugin list command. Then uninstalled it using the plugin uninstall command and again verified the results using the plugin list command.

@tmalikconfluent tmalikconfluent requested a review from a team as a code owner May 13, 2024 23:04
@brianstrauch
Copy link
Member

brianstrauch commented May 13, 2024

Added Plugin uninstall command

Make sure you:

  1. Start this release note with a dash "-" (bullet point)
  2. Format the full command name with backticks so it looks like confluent plugin install
  3. Specify that this command is only for Confluent Platform (I'm pretty sure this is not compatible with Confluent Cloud)

@sgagniere
Copy link
Member

@brianstrauch The plugin command is for both Cloud and Platform (the connect plugin install is Platform only though)

@brianstrauch
Copy link
Member

@brianstrauch The plugin command is for both Cloud and Platform (the connect plugin install is Platform only though)

Since connect plugin install is platform-only, it makes sense that uninstall is also platform-only, right?

@tmalikconfluent
Copy link
Contributor Author

the uninstall works very similar to delete functionality for various other commands, maybe that is why we need it for both cloud and platform? Also have made the other changes you suggested

internal/plugin/command_list.go Outdated Show resolved Hide resolved
internal/plugin/command_list.go Outdated Show resolved Hide resolved
internal/plugin/command_search.go Outdated Show resolved Hide resolved
internal/plugin/command_uninstall.go Outdated Show resolved Hide resolved
test/plugin_test.go Outdated Show resolved Hide resolved
@brianstrauch
Copy link
Member

the uninstall works very similar to delete functionality for various other commands, maybe that is why we need it for both cloud and platform? Also have made the other changes you suggested

Hmm 🤔 since we're only able to install plugins locally with Confluent Platform, I don't think giving Cloud users the option to uninstall would serve any purpose.

@tmalikconfluent tmalikconfluent changed the base branch from main to v4 May 14, 2024 18:39
@tmalikconfluent tmalikconfluent requested a review from a team as a code owner May 14, 2024 18:39
@tmalikconfluent tmalikconfluent changed the base branch from v4 to main May 14, 2024 18:39
@tmalikconfluent tmalikconfluent removed the request for review from a team May 14, 2024 18:44
@tmalikconfluent tmalikconfluent changed the title Add plugin uninstall command [CLI-2908] Add plugin uninstall command May 15, 2024
@sgagniere sgagniere changed the base branch from main to v4 May 15, 2024 19:18
internal/plugin/command_install.go Outdated Show resolved Hide resolved
internal/plugin/command_list.go Outdated Show resolved Hide resolved
@brianstrauch
Copy link
Member

Since this PR is against the v4 branch, I assume there's at least one breaking change. Can you include that in the release notes section and then work with @sgagniere to get that added to the main v4 PR release notes?

@tmalikconfluent tmalikconfluent merged commit 64d79e2 into v4 May 29, 2024
2 checks passed
@tmalikconfluent tmalikconfluent deleted the CLI-2908 branch May 29, 2024 15:23
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.

None yet

3 participants