refactor: move network command into a plugin #3306

tbruyelle · 2022-12-15T19:02:54Z

Resolve #3144

The PR removes completely the network code and thanks to a couple other removals, removes the tendermint/spn dependency from the CLI.

The network code has been moved into a plugin, see related PR ignite/cli-plugin-network#1

In order to make the change as smooth as possible for the user, unlike other plugins, the ignite network or ignite n command is still present in the available commands. I added a concept of "default plugins", which is a list of trusted plugins for which we want to have the related command available, even if the user hasn't fetched and installed the plugin yet.

So the "network" command has been added in this "default plugins" list (more can be added afterward, it should work the same), and as a result, when ignite is run without args, the network sub-command is listed.

Now the cool part of this is when the user runs ignite network (or ignite n chain or whatever sub commands), the attached command will act as a proxy that will add the plugin to the global config, fetch and compile it, and then run it immediately. The only difference for the user is these preliminary steps, which by the way happen only the first time the user runs the ignite network command. Once the plugin is installed, it's as fast as before.

From now, everything related to network commands can be tested, like the usual demo, it should be a good way to ensure nothing is broken. Just compile ignite from this branch and play with it.

and couple of other changes to accomodate plugin network code.

Declare cli-plugin-network as a default plugin.

ignite/cmd/plugin_default.go

ignite/config/plugins/config.go

ignite/pkg/cosmoscmd/root.go

ignite/cmd/plugin_default.go

tbruyelle · 2023-01-03T14:58:51Z

@tbruyelle conflicts

fixed

aljo242 · 2023-01-03T16:11:03Z

https://github.com/ignite/cli/actions/runs/3830513402/jobs/6518479544#step:5:1744

tbruyelle · 2023-01-03T16:12:41Z

https://github.com/ignite/cli/actions/runs/3830513402/jobs/6518479544#step:5:1744

should be OK after my last commit I hope, this was introduced by the merge.

lumtis · 2023-01-04T14:09:06Z

I'm not sure how long you are supposed to wait but it seems like I'm stuck when running the command

ignite network chain list
◤ Building plugin "github.com/ignite/cli-plugin-network@main"

EDIT: The command fails with an error after some time

Also, I have no prompt and stuck when not running a final command (example: just ignite network). Is it supposed to be the case?

ignite network

tbruyelle · 2023-01-04T14:18:43Z

I'm not sure how long you are supposed to wait but it seems like I'm stuck when running the command
ignite network chain list
◤ Building plugin "github.com/ignite/cli-plugin-network@main"
EDIT: The command fails with an error after some time

Can you detail the error please, on my side the command works well.

Also, I have no prompt and stuck when not running a final command (example: just ignite network). Is it supposed to be the case?
ignite network

I can reproduce, that's a regression, I'm on it.

tbruyelle · 2023-01-04T14:24:54Z

@lubtd Please recompile ignite with the latest commit and retry, you should have more meaningful output.

lumtis · 2023-01-04T14:26:04Z

Can you detail the error please, on my side the command works well.

I get

	error: RPC failed; curl 56 LibreSSL SSL_read: SSL_ERROR_SYSCALL, errno 54
	fatal: the remote end hung up unexpectedly
	fatal: early EOF
	fatal: index-pack failed

but I realize I had unstable connectivity, let me retry

How long does it take to you to get the plugin built?

lumtis

It works now @tbruyelle 👍

Plugin build process takes several minutes though, is it normal?

tbruyelle · 2023-01-04T14:45:23Z

It works now @tbruyelle +1

Plugin build process takes several minutes though, is it normal?

On my machine it only takes a few seconds, that might be related to cli-plugin-network dependency to CLI, which is not a tag for the moment, so the build has to find the version and due to the size of the CLI repo, it takes time.

If you rm ~/.ignite/plugins and try again, it's faster no ? (because the CLI dependency is now cached into $GOPATH/pkg)

aljo242 · 2023-01-04T14:46:32Z

It works now @tbruyelle +1
Plugin build process takes several minutes though, is it normal?

On my machine it only takes a few seconds, that might be related to cli-plugin-network dependency to CLI, which is not a tag for the moment, so the build has to find the version and due to the size of the CLI repo, it takes time.

If you rm ~/.ignite/plugins and try again, it's faster no ?

Yes, if I remove and rebuild, its significantly faster!

tbruyelle · 2023-01-04T15:00:28Z

Yes, if I remove and rebuild, its significantly faster!

OK, so that's the reason. I can reproduce the long build by running the following commands :

$ rm -rf ~/.ignite/plugins
$ go clean -modcache
$ ignite network

Unfortunately GOSUMDB=off has already been set when the plugin is built, so I don't know other improvement than using a tag dependency in cli-plugin-network.

aljo242 · 2023-01-04T17:30:40Z

@jeronimoalbi do you think you could review/approve?

* wip: remove network cmd declaration and couple of other changes to accomodate plugin network code. * fix: allow plugin.Use to have more than one word * fix missing feed OSArg * remove appd start-with-http-tunneling command * comment * chore: remove spn dependency from cli * attempt to add legacy plugin command * fix after rebase * feat: add plugin proxy command for default plugins Declare cli-plugin-network as a default plugin. * test: add case for ensureDefaultPlugins * test: add for Config.HasPlugin * chore: add comment * add CL * fix linter * fix: plugin add doesn't kill the plugin on stop * docs: move CL entry to changes * fix network integration tests The network integration tests used to fetch the SPN version from CLI go.mod, but this is no longer possible since the dependency has been removed. To fix that, the SPN version is now read from the cli-plugin-network's go.mod which is cloned from the declared version in defaultPlugins. A fix has also been added to support persistant flags. Finally pluginsconfig.HasPlugin has been updated so it can return true if a local plugin with a module name equals to the given path exists. This permits to use `ignite` with a local cli-plugin-network. * fix lint * set networl plugin version after merge * add network plugin command to gen-cli-docs TODO: need to handle hidden commands * fix linter * fix test * fix: pass events.Bus to plugin.Load Co-authored-by: Alex Johnson <[email protected]> Co-authored-by: Danilo Pantani <[email protected]>

tbruyelle added 14 commits December 14, 2022 17:37

wip: remove network cmd declaration

f2dc442

and couple of other changes to accomodate plugin network code.

fix: allow plugin.Use to have more than one word

f6110ed

fix missing feed OSArg

f1c3ae5

remove appd start-with-http-tunneling command

261b885

comment

25b5b4c

chore: remove spn dependency from cli

464831f

attempt to add legacy plugin command

eca6640

fix after rebase

63c7c7b

Merge branch 'main' into feat/network-plugin

38da0b4

feat: add plugin proxy command for default plugins

7083ae6

Declare cli-plugin-network as a default plugin.

Merge branch 'main' into feat/network-plugin

b28a172

test: add case for ensureDefaultPlugins

a9ad9aa

test: add for Config.HasPlugin

b54afd2

chore: add comment

2a3baee

tbruyelle requested review from lumtis, jeronimoalbi, aljo242, fadeev and Pantani as code owners December 15, 2022 19:02

tbruyelle changed the title ~~feat: network plugin~~ feat: move network code into a plugin Dec 15, 2022

tbruyelle changed the title ~~feat: move network code into a plugin~~ feat: move network command into a plugin Dec 15, 2022

add CL

d9b5716

tbruyelle mentioned this pull request Dec 15, 2022

feat: plugin code and import from cli ignite/cli-plugin-network#1

Merged

tbruyelle commented Dec 15, 2022

View reviewed changes

ignite/cmd/plugin_default.go Outdated Show resolved Hide resolved

tbruyelle added 3 commits December 15, 2022 20:30

fix linter

df0912f

fix: plugin add doesn't kill the plugin on stop

5777063

Merge branch 'main' into feat/network-plugin

14717a8

jeronimoalbi reviewed Dec 16, 2022

View reviewed changes

ignite/config/plugins/config.go Show resolved Hide resolved

jeronimoalbi reviewed Dec 16, 2022

View reviewed changes

ignite/pkg/cosmoscmd/root.go Show resolved Hide resolved

jeronimoalbi reviewed Dec 16, 2022

View reviewed changes

ignite/cmd/plugin_default.go Show resolved Hide resolved

Merge branch 'main' into feat/network-plugin

8b1af57

tbruyelle dismissed aljo242’s stale review via 8b1af57 January 3, 2023 14:58

fix test

73e7571

Alex Johnson added 2 commits January 3, 2023 11:15

Merge branch 'main' into feat/network-plugin

3ee3b0b

Merge branch 'main' into feat/network-plugin

48ac517

aljo242 requested review from jeronimoalbi, lumtis and aljo242 January 4, 2023 13:57

fix: pass events.Bus to plugin.Load

1927c60

lumtis approved these changes Jan 4, 2023

View reviewed changes

tbruyelle and others added 2 commits January 4, 2023 16:00

Merge branch 'main' into feat/network-plugin

59c3a63

Merge branch 'main' into feat/network-plugin

480bc99

aljo242 approved these changes Jan 4, 2023

View reviewed changes

Merge branch 'main' into feat/network-plugin

530aaa7

Merge branch 'main' into feat/network-plugin

a91f639

tbruyelle merged commit a712f70 into main Jan 4, 2023

tbruyelle deleted the feat/network-plugin branch January 4, 2023 18:02

tbruyelle mentioned this pull request Feb 6, 2024

plugins: add support for adding default plugins #3926

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: move network command into a plugin #3306

refactor: move network command into a plugin #3306

tbruyelle commented Dec 15, 2022 •

edited

Loading

tbruyelle commented Jan 3, 2023

aljo242 commented Jan 3, 2023

tbruyelle commented Jan 3, 2023

lumtis commented Jan 4, 2023 •

edited

Loading

tbruyelle commented Jan 4, 2023

tbruyelle commented Jan 4, 2023

lumtis commented Jan 4, 2023

lumtis left a comment

tbruyelle commented Jan 4, 2023 •

edited

Loading

aljo242 commented Jan 4, 2023

tbruyelle commented Jan 4, 2023

aljo242 commented Jan 4, 2023

refactor: move network command into a plugin #3306

refactor: move network command into a plugin #3306

Conversation

tbruyelle commented Dec 15, 2022 • edited Loading

tbruyelle commented Jan 3, 2023

aljo242 commented Jan 3, 2023

tbruyelle commented Jan 3, 2023

lumtis commented Jan 4, 2023 • edited Loading

tbruyelle commented Jan 4, 2023

tbruyelle commented Jan 4, 2023

lumtis commented Jan 4, 2023

lumtis left a comment

Choose a reason for hiding this comment

tbruyelle commented Jan 4, 2023 • edited Loading

aljo242 commented Jan 4, 2023

tbruyelle commented Jan 4, 2023

aljo242 commented Jan 4, 2023

tbruyelle commented Dec 15, 2022 •

edited

Loading

lumtis commented Jan 4, 2023 •

edited

Loading

tbruyelle commented Jan 4, 2023 •

edited

Loading