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

Allow plugin removal with full name, or represent plugin list differently #20668

Closed
jaymode opened this issue Sep 27, 2016 · 6 comments · Fixed by #20807
Closed

Allow plugin removal with full name, or represent plugin list differently #20668

jaymode opened this issue Sep 27, 2016 · 6 comments · Fixed by #20807
Labels
:Core/Infra/Plugins Plugin API and infrastructure discuss

Comments

@jaymode
Copy link
Member

jaymode commented Sep 27, 2016

Filing on behalf of @markwalkom

Elasticsearch version: 5.0.0-beta1

Plugins installed: x-pack

Description of the problem including expected versus actual behavior:
The output of the plugin list command can be counter-intuitive for users looking to remove a plugin:

Current Output:

root@elastic5:~# /usr/share/elasticsearch/bin/elasticsearch-plugin list
x-pack@5.0.0-beta1
root@elastic5:~# /usr/share/elasticsearch/bin/elasticsearch-plugin remove x-pack@5.0.0-beta1
-> Removing x-pack@5.0.0-beta1...
A tool for managing installed elasticsearch plugins

Commands
--------
list - Lists installed elasticsearch plugins
install - Install a plugin
remove - Removes a plugin from elasticsearch

Non-option arguments:
command

Option         Description
------         -----------
-h, --help     show help
-s, --silent   show minimal output
-v, --verbose  show verbose output
ERROR: plugin x-pack@5.0.0-beta1 not found; run 'elasticsearch-plugin list' to get list of installed plugin

Users should be able to remove plugins using the displayed, long name, ie x-pack@5.0.0-beta1, or we should have a columnar representation of the plugins like:

root@elastic5:~# /usr/share/elasticsearch/bin/elasticsearch-plugin list
Name     Version
x-pack    x-pack@5.0.0-beta1
@dadoonet
Copy link
Member

For the record, this has been introduced by #18683

@clintongormley clintongormley added the :Core/Infra/Plugins Plugin API and infrastructure label Oct 7, 2016
@colings86
Copy link
Contributor

Discussed in FixItFriday and we felt that we should use the columnar output for the list command but not prepend the version with the plugin name, so the output would look like:

root@elastic5:~# /usr/share/elasticsearch/bin/elasticsearch-plugin list
Name     Version
x-pack    5.0.0-beta1

We should make sure this change is also made in kibana and logstash so the output is consistent across all three

@colings86 colings86 added help wanted adoptme and removed discuss labels Oct 7, 2016
@dadoonet
Copy link
Member

dadoonet commented Oct 7, 2016

Sorry was not in FixItFriday today and missed that discussion. I wonder if we should be consistent with the _cat API? I mean having optional headers, and by default don't display the version so we keep the previous rendering which is super easy to consume by a shell script and let the users ask for more details with the verbose option?

@jasontedor
Copy link
Member

@colings86 The reason we are here is because of #18683 which wanted a machine-readable format for this output. I'm not sure if the whitespace satisfies that request or not; I'm not sure if we, for example, enforce that the name and version do not contain spaces. We previously discussed #18683 in an earlier Fix-it-Friday; I was against that change because it's conflating a human-readable API with a machine-readable API.

@colings86 colings86 added discuss and removed help wanted adoptme labels Oct 7, 2016
@colings86
Copy link
Contributor

@jasontedor Ok, thanks for giving more background. I have removed adoptme and marks as discuss again as there is obviously more that we need to work out here before we implement/don't implement this change

@rjernst
Copy link
Member

rjernst commented Oct 7, 2016

I also was against the change in #18683. I think it should be reverted. I dont' think we should complicate the plugin tool anymore. We have a verbose option for the extra information. As I explained in that issue, having the version does not actually help with "automating". When elasticsearch is upgraded, all plugins must be upgraded.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Plugins Plugin API and infrastructure discuss
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants