[Merged by Bors] - feat: `fvm show` and `fvm current` command #3601

EstebanBorai · 2023-10-16T18:29:13Z

Introduces two utility commands for FVM used to check on the active version and
listing installed versions.

Use fvm current to print the active Fluvio Version and use fvm show to list
installed Fluvio versions.

Demo

CleanShot.2023-10-16.at.15.44.58.mp4

EstebanBorai · 2023-10-16T18:48:32Z

crates/fluvio-hub-util/src/fvm/mod.rs

-        let ver_b = Channel::parse("0.10.13").unwrap();
+        let ver_b = Channel::parse("0.10.13-mirroring347239873+20231016").unwrap();


Update test to validate tags support.

crates/fluvio-version-manager/fixtures/version/0.10.15/manifest.json

crates/fluvio-version-manager/fixtures/version/stable/manifest.json

sehz

Overall look reasonable. Couple of questions on why additional info for manifest is necessary.

sehz · 2023-10-16T19:00:01Z

crates/fluvio-version-manager/src/command/show.rs

+    /// If theres any Active Version, the [`VersionManifest`] is not included
+    /// as part of the first tuple value, instead it is returned as the second
+    /// tuple value.
+    fn scan_versions_manifests(


this should be moved to version manager or version directory?

I also thought about it but I as well, but I found the use case strongly tied to the purpose of fvm show.

Given that this function not only takes the manifest from VersionDirectory in the process of walking a directory, but it also puts apart the active version, so fvm show can render the check symbol later on.

I could move tho scan logic to be part of VersionDirectory and return the vector filled with all version directories, but then I would have to loop into this vector a second time to retrieve the active version and split it from the rest to provide it to fn render_table(manifests: Vec<VersionManifest>, maybe_active: Option<VersionManifest>).

Issue is that this now has knowledge of internals of the versions, manifest. This needs to move there

Got it! Migrating to VersionDirectory

Here it is! 8c1db1c

ajhunyady · 2023-10-16T19:05:16Z

@EstebanBorai after install, fvm should automatically switch to the version installed. The video shows that as a manual step. This should be true for the first version installed (stable).

Not sure about subsequent versions @sehz ?

sehz · 2023-10-16T19:08:35Z

@EstebanBorai after installation, active version should be stable by default. Also, after installing new version, that version should be set as active. Current demo requires explicit switch

EstebanBorai · 2023-10-16T19:35:32Z

Thanks for asking @ajhunyady and @sehz!

In my video Im showing commands with the build of FVM I had locally. In production users will have FVM installed via our cURL install.sh script which will also install Fluvio and set it up as active by default.

In practice, users running:

curl -fsS https://hub.infinyon.cloud/install/install.sh | bash

Will:

Have fvm installed
Have fluvio stable installed and set as active (Equivalent to fvm install && fvm switch stable)

With regards to setting as default after install, initially we designed a "no side effects" implementation which makes every command atomic.

But I think we could introduce fvm install --switch/-s <VERSION>? Which installs and sets that version as active? I could work a separate iteration for that as it needs some work to make fvm switch command logic reusable by fvm install.

ajhunyady · 2023-10-16T19:38:17Z

install should perform switch under the hood. You can do --no-switch if required, but I don't see a use case for that.

This feedback is on the usability not internal design decisions. Note, usability comes first, and the appropriate design should follow.

sehz · 2023-10-16T21:02:55Z

crates/fluvio-version-manager/src/command/show.rs

+    /// If theres any Active Version, the [`VersionManifest`] is not included
+    /// as part of the first tuple value, instead it is returned as the second
+    /// tuple value.
+    fn scan_versions_manifests(


Issue is that this now has knowledge of internals of the versions, manifest. This needs to move there

sehz · 2023-10-16T21:05:19Z

But I think we could introduce fvm install --switch/-s <VERSION>? Which installs and sets that version as active? I could work a separate iteration for that as it needs some work to make fvm switch command logic reusable by fvm install.

Yes, that can be done in separate PR. But switch flag is not necessary. Also assume switch to installed version

sehz

K. LGTM. Please update install logic to switch to version in next PR

ajhunyady · 2023-10-16T22:00:28Z

@EstebanBorai please file a new issue with left-over items and add to tracking issue #3537 so we don't lose them.

EstebanBorai · 2023-10-17T00:10:44Z

bors r+

Introduces two utility commands for FVM used to check on the active version and listing installed versions. Use `fvm current` to print the active Fluvio Version and use `fvm show` to list installed Fluvio versions. ## Demo https://github.com/infinyon/fluvio/assets/34756077/a2327a63-1d9e-4ceb-86d3-02522de0aa42

bors · 2023-10-17T00:44:14Z

Build failed:

Done

morenol · 2023-10-17T00:58:14Z

bors r+

Introduces two utility commands for FVM used to check on the active version and listing installed versions. Use `fvm current` to print the active Fluvio Version and use `fvm show` to list installed Fluvio versions. ## Demo https://github.com/infinyon/fluvio/assets/34756077/a2327a63-1d9e-4ceb-86d3-02522de0aa42

bors · 2023-10-17T01:21:07Z

Pull request successfully merged into master.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

Done

EstebanBorai added 3 commits October 16, 2023 14:58

feat: fvm show command for listing versions

f9c9216

feat: fvm current command for active version

4f16924

feat: message for empty version list on fvm show

8f4554f

EstebanBorai requested review from digikata and sehz October 16, 2023 18:48

EstebanBorai marked this pull request as ready for review October 16, 2023 18:48

EstebanBorai commented Oct 16, 2023

View reviewed changes

This was linked to issues Oct 16, 2023

FVM - List Installed Versions #3599

Closed

FVM - Display active version #3600

Closed

sehz reviewed Oct 16, 2023

View reviewed changes

crates/fluvio-version-manager/fixtures/version/0.10.15/manifest.json Show resolved Hide resolved

sehz reviewed Oct 16, 2023

View reviewed changes

crates/fluvio-version-manager/fixtures/version/stable/manifest.json Show resolved Hide resolved

sehz requested changes Oct 16, 2023

View reviewed changes

EstebanBorai requested a review from sehz October 16, 2023 19:44

sehz requested changes Oct 16, 2023

View reviewed changes

chore: move scan_versions_manifests into VersionManifest impl

8c1db1c

sehz approved these changes Oct 16, 2023

View reviewed changes

EstebanBorai mentioned this pull request Oct 16, 2023

FVM - Set installed version as active #3602

Closed

EstebanBorai linked an issue Oct 16, 2023 that may be closed by this pull request

Fluvio Version Manager #3537

Closed

20 tasks

ajhunyady mentioned this pull request Oct 16, 2023

Fluvio Version Manager #3537

Closed

20 tasks

bors bot changed the title ~~feat: fvm show and fvm current command~~ [Merged by Bors] - feat: fvm show and fvm current command Oct 17, 2023

bors bot closed this Oct 17, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Merged by Bors] - feat: `fvm show` and `fvm current` command #3601

[Merged by Bors] - feat: `fvm show` and `fvm current` command #3601

EstebanBorai commented Oct 16, 2023 •

edited

EstebanBorai Oct 16, 2023

sehz left a comment

sehz Oct 16, 2023

EstebanBorai Oct 16, 2023

sehz Oct 16, 2023

EstebanBorai Oct 16, 2023

EstebanBorai Oct 16, 2023

ajhunyady commented Oct 16, 2023 •

edited

sehz commented Oct 16, 2023 •

edited

EstebanBorai commented Oct 16, 2023

ajhunyady commented Oct 16, 2023 •

edited

sehz Oct 16, 2023

sehz commented Oct 16, 2023 •

edited

sehz left a comment

ajhunyady commented Oct 16, 2023 •

edited

EstebanBorai commented Oct 17, 2023

bors bot commented Oct 17, 2023

morenol commented Oct 17, 2023

bors bot commented Oct 17, 2023

		let ver_b = Channel::parse("0.10.13").unwrap();
		let ver_b = Channel::parse("0.10.13-mirroring347239873+20231016").unwrap();

[Merged by Bors] - feat: fvm show and fvm current command #3601

[Merged by Bors] - feat: fvm show and fvm current command #3601

Conversation

EstebanBorai commented Oct 16, 2023 • edited

Demo

EstebanBorai Oct 16, 2023

Choose a reason for hiding this comment

sehz left a comment

Choose a reason for hiding this comment

sehz Oct 16, 2023

Choose a reason for hiding this comment

EstebanBorai Oct 16, 2023

Choose a reason for hiding this comment

sehz Oct 16, 2023

Choose a reason for hiding this comment

EstebanBorai Oct 16, 2023

Choose a reason for hiding this comment

EstebanBorai Oct 16, 2023

Choose a reason for hiding this comment

ajhunyady commented Oct 16, 2023 • edited

sehz commented Oct 16, 2023 • edited

EstebanBorai commented Oct 16, 2023

ajhunyady commented Oct 16, 2023 • edited

sehz Oct 16, 2023

Choose a reason for hiding this comment

sehz commented Oct 16, 2023 • edited

sehz left a comment

Choose a reason for hiding this comment

ajhunyady commented Oct 16, 2023 • edited

EstebanBorai commented Oct 17, 2023

bors bot commented Oct 17, 2023

morenol commented Oct 17, 2023

bors bot commented Oct 17, 2023

[Merged by Bors] - feat: `fvm show` and `fvm current` command #3601

[Merged by Bors] - feat: `fvm show` and `fvm current` command #3601

EstebanBorai commented Oct 16, 2023 •

edited

ajhunyady commented Oct 16, 2023 •

edited

sehz commented Oct 16, 2023 •

edited

ajhunyady commented Oct 16, 2023 •

edited

sehz commented Oct 16, 2023 •

edited

ajhunyady commented Oct 16, 2023 •

edited