Skip to content

Commit

Permalink
plugin install: don't print download progress in batch mode (#36361)
Browse files Browse the repository at this point in the history
* Don't print download progress in batch mode

With this change we will no longer provide the progress bar in batch
mode.
Assuming that this is mode is mainly for consumption by tools which
will serialize the output, we shouldn't print a progress bar to be
for every percentile.

* PR review
  • Loading branch information
alpar-t committed Dec 12, 2018
1 parent 0caa003 commit 8d3dec5
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ void execute(Terminal terminal, String pluginId, boolean isBatch, Environment en
handleInstallXPack(buildFlavor());
}

Path pluginZip = download(terminal, pluginId, env.tmpFile());
Path pluginZip = download(terminal, pluginId, env.tmpFile(), isBatch);
Path extractedZip = unzip(pluginZip, env.pluginsFile());
install(terminal, isBatch, extractedZip, env);
}
Expand All @@ -249,19 +249,19 @@ private static void handleInstallXPack(final Build.Flavor flavor) throws UserExc
}

/** Downloads the plugin and returns the file it was downloaded to. */
private Path download(Terminal terminal, String pluginId, Path tmpDir) throws Exception {
private Path download(Terminal terminal, String pluginId, Path tmpDir, boolean isBatch) throws Exception {
if (OFFICIAL_PLUGINS.contains(pluginId)) {
final String url = getElasticUrl(terminal, getStagingHash(), Version.CURRENT, isSnapshot(), pluginId, Platforms.PLATFORM_NAME);
terminal.println("-> Downloading " + pluginId + " from elastic");
return downloadAndValidate(terminal, url, tmpDir, true);
return downloadAndValidate(terminal, url, tmpDir, true, isBatch);
}

// now try as maven coordinates, a valid URL would only have a colon and slash
String[] coordinates = pluginId.split(":");
if (coordinates.length == 3 && pluginId.contains("/") == false && pluginId.startsWith("file:") == false) {
String mavenUrl = getMavenUrl(terminal, coordinates, Platforms.PLATFORM_NAME);
terminal.println("-> Downloading " + pluginId + " from maven central");
return downloadAndValidate(terminal, mavenUrl, tmpDir, false);
return downloadAndValidate(terminal, mavenUrl, tmpDir, false, isBatch);
}

// fall back to plain old URL
Expand All @@ -275,7 +275,7 @@ private Path download(Terminal terminal, String pluginId, Path tmpDir) throws Ex
throw new UserException(ExitCodes.USAGE, msg);
}
terminal.println("-> Downloading " + URLDecoder.decode(pluginId, "UTF-8"));
return downloadZip(terminal, pluginId, tmpDir);
return downloadZip(terminal, pluginId, tmpDir, isBatch);
}

// pkg private so tests can override
Expand Down Expand Up @@ -370,14 +370,14 @@ private List<String> checkMisspelledPlugin(String pluginId) {
/** Downloads a zip from the url, into a temp file under the given temp dir. */
// pkg private for tests
@SuppressForbidden(reason = "We use getInputStream to download plugins")
Path downloadZip(Terminal terminal, String urlString, Path tmpDir) throws IOException {
Path downloadZip(Terminal terminal, String urlString, Path tmpDir, boolean isBatch) throws IOException {
terminal.println(VERBOSE, "Retrieving zip from " + urlString);
URL url = new URL(urlString);
Path zip = Files.createTempFile(tmpDir, null, ".zip");
URLConnection urlConnection = url.openConnection();
urlConnection.addRequestProperty("User-Agent", "elasticsearch-plugin-installer");
int contentLength = urlConnection.getContentLength();
try (InputStream in = new TerminalProgressInputStream(urlConnection.getInputStream(), contentLength, terminal)) {
try (InputStream in = isBatch ? urlConnection.getInputStream() :
new TerminalProgressInputStream(urlConnection.getInputStream(),urlConnection.getContentLength(),terminal)) {
// must overwrite since creating the temp file above actually created the file
Files.copy(in, zip, StandardCopyOption.REPLACE_EXISTING);
}
Expand Down Expand Up @@ -440,17 +440,18 @@ private InputStream urlOpenStream(final URL url) throws IOException {
* @param urlString the URL of the plugin ZIP
* @param tmpDir a temporary directory to write downloaded files to
* @param officialPlugin true if the plugin is an official plugin
* @param isBatch true if the install is running in batch mode
* @return the path to the downloaded plugin ZIP
* @throws IOException if an I/O exception occurs download or reading files and resources
* @throws PGPException if an exception occurs verifying the downloaded ZIP signature
* @throws UserException if checksum validation fails
*/
private Path downloadAndValidate(
final Terminal terminal,
final String urlString,
final Path tmpDir,
final boolean officialPlugin) throws IOException, PGPException, UserException {
Path zip = downloadZip(terminal, urlString, tmpDir);
final Terminal terminal,
final String urlString,
final Path tmpDir,
final boolean officialPlugin, boolean isBatch) throws IOException, PGPException, UserException {
Path zip = downloadZip(terminal, urlString, tmpDir, isBatch);
pathsToDeleteOnShutdown.add(zip);
String checksumUrlString = urlString + ".sha512";
URL checksumUrl = openUrl(checksumUrlString);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -797,6 +797,9 @@ public void testBatchFlag() throws Exception {
MockTerminal terminal = new MockTerminal();
installPlugin(terminal, true);
assertThat(terminal.getOutput(), containsString("WARNING: plugin requires additional permissions"));
assertThat(terminal.getOutput(), containsString("-> Downloading"));
// No progress bar in batch mode
assertThat(terminal.getOutput(), not(containsString("100%")));
}

public void testQuietFlagDisabled() throws Exception {
Expand Down Expand Up @@ -852,7 +855,7 @@ void assertInstallPluginFromUrl(
Path pluginZip = createPlugin(name, pluginDir);
InstallPluginCommand command = new InstallPluginCommand() {
@Override
Path downloadZip(Terminal terminal, String urlString, Path tmpDir) throws IOException {
Path downloadZip(Terminal terminal, String urlString, Path tmpDir, boolean isBatch) throws IOException {
assertEquals(url, urlString);
Path downloadedPath = tmpDir.resolve("downloaded.zip");
Files.copy(pluginZip, downloadedPath);
Expand Down

0 comments on commit 8d3dec5

Please sign in to comment.