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

Plugins: Add status bar on download #18695

Merged

Conversation

spinscale
Copy link
Contributor

As some plugins are becoming biggish now, it is hard for the user to know, if the plugin
is being downloaded or just nothing happens. I also had some cases running on a VM where I did not know if the networking was broken or still downloading, while testing.

This commit adds a progress bar during download, which can be disabled by using the -q
parameter - it is pretty much unstyled right now, but at least shows an indicator that things are happening.

In addition this updates to jimfs 1.1, which allows us to test the batch mode, as adding
security policies are now supported due to having jimfs:// protocol support in URL stream
handlers.

TODO: Update docs
TODO: Test on windows (only tested under osx/linux so far)

This is how it looks like right now

Video demo

@dadoonet
Copy link
Member

dadoonet commented Jun 2, 2016

Looks great so far @spinscale!

@clintongormley clintongormley added the :Core/Infra/Plugins Plugin API and infrastructure label Jun 2, 2016
@Override
public int read() throws IOException {
int read = in.read();
checkProgress(read == -1 ? -1 : 1);
Copy link
Member

Choose a reason for hiding this comment

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

Why is the ternary needed since checkProgress handles -1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in.read() returns the next byte of data and not the length of the data being read, like the other to read() methods

Copy link
Member

Choose a reason for hiding this comment

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

Ah of course. Ignore my brain fart.

@rjernst
Copy link
Member

rjernst commented Jun 2, 2016

@spinscale I left some comments.

@spinscale
Copy link
Contributor Author

incorporated your review comments, thx for the review!

/** Prints message to the terminal at {@code verbosity} level, without a newline. */
public final void print(Verbosity verbosity, String msg) {
if (this.verbosity.ordinal() >= verbosity.ordinal()) {
getWriter().print(msg);
Copy link
Member

Choose a reason for hiding this comment

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

I think println should be rewritten to use this?

public final void println(Verbosity verbosity, String msg) {
    print(verbosity, msg + lineSeparator);
}

@rjernst
Copy link
Member

rjernst commented Jun 14, 2016

I left a couple more comments. LGTM otherwise.

@dadoonet
Copy link
Member

dadoonet commented Jun 15, 2016

I checked it on a windows box and it gives the following results:

PS C:\Users\IEUser\Downloads\elasticsearch-5.0.0-alpha4-SNAPSHOT> .\bin\elasticsearch-plugin install https://download.elastic.co/elasticsearch/release/org/elasticsearch/plugin/repository-s3/5.0.0-alpha3/repository-s3-5.0.0-alpha3.zip
-> Downloading https://download.elastic.co/elasticsearch/release/org/elasticsearch/plugin/repository-s3/5.0.0-alpha3/repository-s3-5.0.0-alpha3.zip
[                                                 ] 1%  
[>                                                ] 2%  
[>                                                ] 3%  
[=>                                               ] 4%  
[=>                                               ] 5%  
[==>                                              ] 6%  
[==>                                              ] 7%  
[===>                                             ] 8%  
[===>                                             ] 9%  
[====>                                            ] 10%  
[====>                                            ] 11%  
[=====>                                           ] 12%  
[=====>                                           ] 13%  
[======>                                          ] 14%  
[======>                                          ] 15%  
[=======>                                         ] 16%  
[=======>                                         ] 17%  
[========>                                        ] 18%  
[========>                                        ] 19%  
[=========>                                       ] 20%  
[=========>                                       ] 21%  
[==========>                                      ] 22%  
[==========>                                      ] 23%  
[===========>                                     ] 24%  
[===========>                                     ] 25%  
[============>                                    ] 26%  
[============>                                    ] 27%  
[=============>                                   ] 28%  
[=============>                                   ] 29%  

Not as nice as on Linux but still working though. :)

@dadoonet
Copy link
Member

dadoonet commented Jun 29, 2016

Tested on windows:

C:\Users\IEUser\Downloads\elasticsearch-5.0.0-alpha4-SNAPSHOT\bin>elasticsearch-plugin install x-pack
-> Downloading x-pack from elastic
[=================================================] 100%  
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
@     WARNING: plugin requires additional permissions     @
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
* java.lang.RuntimePermission accessClassInPackage.com.sun.activation.registries
* java.lang.RuntimePermission getClassLoader
* java.lang.RuntimePermission setContextClassLoader
* java.lang.RuntimePermission setFactory
* java.security.SecurityPermission createPolicy.JavaPolicy
* java.security.SecurityPermission getPolicy
* java.security.SecurityPermission putProviderProperty.BC
* java.security.SecurityPermission setPolicy
* java.util.PropertyPermission * read,write
* javax.net.ssl.SSLPermission setHostnameVerifier
See http://docs.oracle.com/javase/8/docs/technotes/guides/security/permissions.html for descriptions of what these permissions allow and the associated risks.

Continue with installation? [y/N]y
-> Installed x-pack

Works well!

@spinscale spinscale force-pushed the 1606-plugin-install-progress-bar branch from 510f5ff to 1d18d5c Compare June 29, 2016 13:26
As some plugins are becoming big now, it is hard for the user to know, if the plugin
is being downloaded or just nothing happens.

This commit adds a progress bar during download, which can be disabled by using the `-q`
parameter.

In addition this updates to jimfs 1.1, which allows us to test the batch mode, as adding
security policies are now supported due to having jimfs:// protocol support in URL stream
handlers.
@spinscale spinscale force-pushed the 1606-plugin-install-progress-bar branch from 1d18d5c to 50bafb5 Compare June 29, 2016 13:26
@spinscale spinscale merged commit 56fa751 into elastic:master Jun 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants