tanzu-cli icon indicating copy to clipboard operation
tanzu-cli copied to clipboard

Plugin installation UX update

Open chandrareddyp opened this issue 1 year ago • 5 comments

What this PR does / why we need it

This PR enhances the plugin installation. After these changes, the plugin installation does not log each successful plugin installation message, but it logs the in-progress installation message. However, it does overwrite the log to display the next plugin installation in-progress message. Additionally, each plugin installation in-progress message includes the [total plugins to install / plugins being installed]. This PR impacts below use cases:

  • Plugin installation (stand-alone and target specific, local source)
  • Plugin sync
  • Plugin context use and create
  • Plugin group installation
  • Plugin upgrade

Which issue(s) this PR fixes

Fixes #

Describe testing done for PR

  1. Essential Plugin installation: image

image

  1. Specific Plugin installation: image image

image

image

  1. Install plugins from a plugin group: image image

image

  1. Plugin sync: image

When its terminated (with Ctl+c) image

Plugin sync completed successfully: image

  1. Context use: When context use being terminated (Ctl+c) image Context use completed successfully: image

  2. Plugin install from local source: tp plugin install builder --version v1.3.0-dev --local-source /Users/cpamuluri/tkg/tasks/code/cli-core2/tanzu-cli/artifacts/plugins/darwin/amd64 image

Release note

The plugin installation UX has been updated to log the total number of plugins to install and the number of plugins being installed as part of the in-progress log message. It also removes the successful installation message for each plugin installation.

Additional information

Special notes for your reviewer

chandrareddyp avatar Mar 14 '24 17:03 chandrareddyp

Thanks!

When installing from a group, the logic uses a different spinner for each plugin. However, each spinner, when closed, adds an empty line to the output. That does not look good:

tz plugin install --group vmware-tkg/default                                                                                  <gke_dev-adhol>
[i] The following plugins will be installed from plugin group 'vmware-tkg/default:v2.5.0'
  NAME                TARGET      VERSION
  isolated-cluster    global      v0.32.1
  management-cluster  kubernetes  v0.32.1
  package             kubernetes  v0.32.1
  pinniped-auth       global      v0.32.1
  secret              kubernetes  v0.32.0
  telemetry           kubernetes  v0.32.1






[ok] successfully installed all plugins from group 'vmware-tkg/default:v2.5.0'

It becomes even worse when creating a TMC context which install 24 plugins, and therefore adds 24 empty lines to the output. a) We can either fix the spinner API to allow turning off the extra empty line. We had discussed this before. b) Or we could use the same spinner and just change its text. Maybe a global spinner can be defined instead of a local one?

Its been fixed, can you use the https://github.com/vmware-tanzu/tanzu-plugin-runtime/pull/170 runtime changes along with this PR.

chandrareddyp avatar Mar 19 '24 14:03 chandrareddyp

I noticed that we are starting count from [0/1] and [0/5] instead of [1/1] and [1/5].

In the testing done section, when installing specific plugins I see

[0/0] Failed to install plugin ...
0 plugins installed out of 0

This doesn't look right to me. It should be [1/1] and 0 plugins installed out of 1. However, I feel that if we are installing just a single plugin we should not be showing 0 plugins installed out of 1 log as well.

anujc25 avatar Apr 30 '24 18:04 anujc25

I noticed that we are starting count from [0/1] and [0/5] instead of [1/1] and [1/5].

In the testing done section, when installing specific plugins I see

[0/0] Failed to install plugin ...
0 plugins installed out of 0

This doesn't look right to me. It should be [1/1] and 0 plugins installed out of 1. However, I feel that if we are installing just a single plugin we should not be showing 0 plugins installed out of 1 log as well.

Whats the use case? @anujc25

chandrareddyp avatar May 01 '24 21:05 chandrareddyp

Related to @anujc25's comment, I still see

| [0/6] Installing plugin 'appsv2:v0.2.0-beta.3' with target 'kubernetes'

This should be [1/6]: we are not installing the 0th plugin out of 6, but the 1st plugin out of 6.

@chandrareddyp I guess your idea was that the first number is the number of plugins already installed? I think it is more common that this number is the step number, so 1/6, 2/6 until 6/6 when we are installing the 6th plugin out of 6.

marckhouzam avatar May 02 '24 20:05 marckhouzam

Related to @anujc25's comment, I still see

| [0/6] Installing plugin 'appsv2:v0.2.0-beta.3' with target 'kubernetes'

This should be [1/6]: we are not installing the 0th plugin out of 6, but the 1st plugin out of 6.

@chandrareddyp I guess your idea was that the first number is the number of plugins already installed? I think it is more common that this number is the step number, so 1/6, 2/6 until 6/6 when we are installing the 6th plugin out of 6.

Yes the first number indicates no.of plugins already installed.

I felt, it make sense to indicate the count of the plugin being installed! i have updated the code. Thanks.

chandrareddyp avatar May 06 '24 12:05 chandrareddyp