GoogleCloudPlatform / appengine-plugins

A client Java library to manage App Engine Java applications for any project that performs App Engine Java application management. For example, the Maven, Gradle and Eclipse App Engine plugins, custom user tools, etc.
Apache License 2.0
40 stars 26 forks source link

Fixing cloud sdk install progress bar display time #871

Closed peterlin741 closed 3 years ago

peterlin741 commented 3 years ago

Summary

image

progress

Ran the following:

Testing

Release Process to Follow

codecov[bot] commented 3 years ago

Codecov Report

Merging #871 (e68e569) into master (8948d05) will decrease coverage by 0.02%. The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #871      +/-   ##
============================================
- Coverage     78.47%   78.44%   -0.03%     
+ Complexity      624      623       -1     
============================================
  Files            96       96              
  Lines          2467     2464       -3     
  Branches        288      287       -1     
============================================
- Hits           1936     1933       -3     
  Misses          421      421              
  Partials        110      110              
Impacted Files Coverage Δ Complexity Δ
...ud/tools/managedcloudsdk/install/SdkInstaller.java 83.05% <100.00%> (-0.83%) 11.00 <0.00> (-1.00)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 8948d05...e68e569. Read the comment docs.

chanseokoh commented 3 years ago

So the way you resolve the issue is making the progress reporting coarse (from explicitly allocating progress units for the entire steps to just saying "uh, don't know how long it will take"). In IntelliJ, as the "unknown" progress work, you probably set up an IDE progress reporting that just rolls (i.e., no incremental progress updates by ticks). So I wonder if it's just that your ProgressListener implementation in IDE is not handling the signals properly? The appengine-plugins-core is just notifying the client through the ProgressListener interface, and I think it should mainly be the responsibility of Cloud Code to interpret the calls in whatever way it wants to.

peterlin741 commented 3 years ago

Agreed, that makes sense.

This is a little tricky, Cloud Code's implementation for installing the sdk (ManagedCloudSdkService::installSdk()) is nearly identical to the code below, for installing components (ManagedCloudSdkService::installComponent()), which already correctly shows the progress indicators.

The only difference I see is that this library's SdkInstaller::install creates child processes. I see in our code base (ManagedCloudSdkProgressListener::newChild) that we use ChildProgressListener, which is defined in this library.

I'm not sure if it's worth digger further to fix this bug, but let me know if you have any ideas at a glance, as to what could be going wrong?

peterlin741 commented 3 years ago

Closing for now. Confirmed that this was working a while ago, so a bug was probably introduced. Will investigate the change logs and reopen

chanseokoh commented 3 years ago

I see. Feel free to restore the branch when needed.