[Merged by Bors] - feat: FVM `self` subcommand #3570

EstebanBorai · 2023-09-30T19:33:43Z

Introduces a subcommand for installing/uninstalling and in the short term also updating
FVM. This command is in charge of preparing the environment for FVM before installing
and switching Fluvio Versions.

Demo

crates/fluvio-version-manager/Cargo.toml

crates/fluvio-version-manager/src/command/itself/install.rs

crates/fluvio-version-manager/src/common/settings.rs

crates/fluvio-version-manager/src/common/workdir.rs

crates/fluvio-version-manager/src/main.rs

sehz · 2023-09-30T20:39:36Z

please add CLI tests in addition to unit test

tests/cli/fvm_smoke_tests/fvm_basic.bats

crates/fluvio-version-manager/src/common/settings.rs

ajhunyady · 2023-10-02T18:46:28Z

@EstebanBorai do we also need an update self command.

crates/fluvio-version-manager/src/common/settings.rs

EstebanBorai · 2023-10-02T19:43:31Z

@EstebanBorai do we also need an update self command.

As of today we will rely on the install script given for updating FVM, in the future we are likely to implement a dedicated approach.

sehz

Still missing CLI / Unit tests.

I would expect following tests:

$ ../fvm self-install

$ .../fvm uninstall

And test to ensure directory is created correctly (install) with stable channel. and test to ensure everything is removed correctly

.github/workflows/ci.yml

sehz · 2023-10-02T22:33:13Z

crates/fluvio-version-manager/src/common/settings.rs

+#[derive(Debug, Serialize, Deserialize)]
+pub struct Settings {
+    /// The active `channel` for the Fluvio Installation
+    pub channel: Option<Channel>,


this needs to be fixed. it is probably most critical part of this PR

sehz · 2023-10-02T22:36:17Z

crates/fluvio-version-manager/src/common/settings.rs

+
+    #[test]
+    fn update_settings_file() {
+        create_fvm_dir();


why this is calling this? shouldn't installation step should take care of it?

This is testing tho Settings file creation only, these are unit tests on the Settings struct

shouldn't installation step should take care of it?
Yeah! The command takes care of this but the Settings struct is not responsible of creating the directory.

In this context we just create the settings.toml file, se I have to prepare the environment for unit tests in this context

EstebanBorai · 2023-10-02T23:37:03Z

Still missing CLI / Unit tests.

I would expect following tests:
$ ../fvm self-install
$ .../fvm uninstall
And test to ensure directory is created correctly (install) with stable channel. and test to ensure everything is removed correctly

I think the tests you are mentioning are here: https://github.com/infinyon/fluvio/blob/8053a155b08ffc92b5e08ee8cae26dd9863c02ac/tests/cli/fvm_smoke_tests/fvm_basic.bats perhaps?

sehz · 2023-10-02T23:40:21Z

These tests doesnt do anything since they dont have any validation

EstebanBorai · 2023-10-03T02:13:15Z

These tests doesnt do anything since they dont have any validation

Oh! Sorry about that! I just updated tests, now I also check the presence of the ~/.fvm/bin/fvm.

Im testing the following assertions:

`fvm self install`

Create the ~/.fvm directory
Create a settings.toml, but as no version of Fluvio has been installed already, the settings.toml remains empty as of this phase
Copies the fvm binary to ~/.fvm/bin/fvm
Prints the command to add fvm to the path for the user

`fvm self uninstall`

Delete the ~/.fvm directory.

Previously I was checking the settings file to be there, is important to keep in mind that fvm self install doesnh have any side effects, no fluvio versions will be installed as of this phase. The settings.toml content is empty as no versions has been installed so far.

In the following iteration, with fvm install, settings.toml will be updated to have the
versions and channels that are active.

EstebanBorai · 2023-10-03T15:32:55Z

@sehz just checked for existence using test command.
Checking for each file to be present, lets see how this goes through CI!

crates/fluvio-version-manager/src/common/settings.rs

tests/cli/fvm_smoke_tests/fvm_basic.bats

crates/fluvio-version-manager/src/common/settings.rs

EstebanBorai · 2023-10-03T21:30:24Z

bors r+

Introduces a subcommand for installing/uninstalling and in the short term also updating FVM. This command is in charge of preparing the environment for FVM before installing and switching Fluvio Versions. --- ## Demo ![CleanShot 2023-09-30 at 19 21 45](https://github.com/infinyon/fluvio/assets/34756077/57fc1a71-a122-4658-b4b8-6e11443a8ceb)

EstebanBorai · 2023-10-03T21:31:12Z

Thanks for your time @sehz and @digikata! Moving into fvm install now!
Refer: #3573

sehz · 2023-10-03T21:35:22Z

There is an issue with the setting config. Please address them on next PR

bors · 2023-10-03T22:09:24Z

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 September 30, 2023 14:52

feat: fvm self command to install/uninstall fvm

6708fec

fix: clippy issues

d5f6a73

feat: add to path capabilities

2d749a5

EstebanBorai requested review from digikata and sehz September 30, 2023 19:33

sehz requested changes Sep 30, 2023

View reviewed changes

EstebanBorai added 6 commits September 30, 2023 18:54

fix: set version 0.0.0 for unpublished

3d26dd8

feat: move dialoguer to workspace

b03d51f

fix: remove surf from fvm

1bdcc5f

feat: use Self* for command names

e068317

feat: use anyhow over eyre

3c5cdbc

chore: tests and pr rview improvements

0ed7b25

EstebanBorai requested a review from sehz October 2, 2023 14:39

sehz requested changes Oct 2, 2023

View reviewed changes

EstebanBorai commented Oct 2, 2023

View reviewed changes

crates/fluvio-version-manager/src/common/settings.rs Show resolved Hide resolved

feat: cli tests

8053a15

EstebanBorai requested a review from sehz October 2, 2023 22:00

sehz requested changes Oct 2, 2023

View reviewed changes

EstebanBorai added 2 commits October 2, 2023 22:51

feat: condense if clause in CI

36d6ffc

feat: check dirs are created

ba8853c

EstebanBorai requested a review from sehz October 3, 2023 02:16

fix: test assertions

500ed0c

digikata reviewed Oct 3, 2023

View reviewed changes

crates/fluvio-version-manager/src/common/settings.rs Show resolved Hide resolved

sehz requested changes Oct 3, 2023

View reviewed changes

tests/cli/fvm_smoke_tests/fvm_basic.bats Show resolved Hide resolved

crates/fluvio-version-manager/src/common/settings.rs Show resolved Hide resolved

EstebanBorai requested review from digikata and sehz October 3, 2023 21:03

digikata approved these changes Oct 3, 2023

View reviewed changes

bors bot changed the title ~~feat: FVM self subcommand~~ [Merged by Bors] - feat: FVM self subcommand Oct 3, 2023

bors bot closed this Oct 3, 2023

sehz added this to the 0.10.16 milestone Oct 7, 2023

sehz added the feature/fvm label Oct 7, 2023

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

FVM - Installation #3569

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Merged by Bors] - feat: FVM `self` subcommand #3570

[Merged by Bors] - feat: FVM `self` subcommand #3570

EstebanBorai commented Sep 30, 2023 •

edited

Loading

sehz commented Sep 30, 2023

ajhunyady commented Oct 2, 2023

EstebanBorai commented Oct 2, 2023

sehz left a comment

sehz Oct 2, 2023

sehz Oct 2, 2023

EstebanBorai Oct 2, 2023 •

edited

Loading

EstebanBorai Oct 3, 2023

EstebanBorai commented Oct 2, 2023

sehz commented Oct 2, 2023

EstebanBorai commented Oct 3, 2023

EstebanBorai commented Oct 3, 2023

EstebanBorai commented Oct 3, 2023

EstebanBorai commented Oct 3, 2023

sehz commented Oct 3, 2023

bors bot commented Oct 3, 2023

[Merged by Bors] - feat: FVM self subcommand #3570

[Merged by Bors] - feat: FVM self subcommand #3570

Conversation

EstebanBorai commented Sep 30, 2023 • edited Loading

Demo

sehz commented Sep 30, 2023

ajhunyady commented Oct 2, 2023

EstebanBorai commented Oct 2, 2023

sehz left a comment

Choose a reason for hiding this comment

sehz Oct 2, 2023

Choose a reason for hiding this comment

sehz Oct 2, 2023

Choose a reason for hiding this comment

EstebanBorai Oct 2, 2023 • edited Loading

Choose a reason for hiding this comment

EstebanBorai Oct 3, 2023

Choose a reason for hiding this comment

EstebanBorai commented Oct 2, 2023

sehz commented Oct 2, 2023

EstebanBorai commented Oct 3, 2023

fvm self install

fvm self uninstall

EstebanBorai commented Oct 3, 2023

EstebanBorai commented Oct 3, 2023

EstebanBorai commented Oct 3, 2023

sehz commented Oct 3, 2023

bors bot commented Oct 3, 2023

[Merged by Bors] - feat: FVM `self` subcommand #3570

[Merged by Bors] - feat: FVM `self` subcommand #3570

EstebanBorai commented Sep 30, 2023 •

edited

Loading

EstebanBorai Oct 2, 2023 •

edited

Loading

`fvm self install`

`fvm self uninstall`