Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: interrupt plugin commands #3222

Merged
merged 3 commits into from
Dec 5, 2022
Merged

fix: interrupt plugin commands #3222

merged 3 commits into from
Dec 5, 2022

Conversation

tbruyelle
Copy link
Contributor

Due to the plugin architecture, the user wasn't able to interrupt a plugin command via Ctrl+C. This is annoying if the plugin execution is long.

By running the plugin execution in a goroutine and by listening to the command context at the same time, we can fix that.

@tbruyelle tbruyelle added the skip-changelog Don't check changelog for new entries label Dec 2, 2022
Due to the plugin architecture, the user wasn't able to interrupt a
plugin command via Ctrl+C. This is annoying if the plugin execution is
long.

By running the plugin execution in a goroutine and by listening to the
command context at the same time, we can fix that.
@tbruyelle tbruyelle force-pushed the fix/interrupt-plugin-cmd branch from 27eac59 to e64a36a Compare December 2, 2022 17:14
Copy link
Member

@jeronimoalbi jeronimoalbi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

It might be a good idea to wrap the RPC calls to add context support to them. I'm not sure how much work that might be but if it's possible we could have an specific issue for that, it would be a nice feature.

@tbruyelle
Copy link
Contributor Author

tbruyelle commented Dec 2, 2022

LGTM +1

It might be a good idea to wrap the RPC calls to add context support to them. I'm not sure how much work that might be but if it's possible we could have an specific issue for that, it would be a nice feature.

Well, w/o modification in hashicorp/go-plugin I don't think this is feasible. I first checked the codebase to see if there was some existing mechanism, I found that PR where the guy concludes the feature is an anti-pattern, but I couldn't understand why.

@codecov
Copy link

codecov bot commented Dec 5, 2022

Codecov Report

Base: 19.94% // Head: 19.97% // Increases project coverage by +0.02% 🎉

Coverage data is based on head (90bca10) compared to base (a00cdf4).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3222      +/-   ##
==========================================
+ Coverage   19.94%   19.97%   +0.02%     
==========================================
  Files         382      382              
  Lines       30101    30108       +7     
==========================================
+ Hits         6004     6014      +10     
+ Misses      23521    23519       -2     
+ Partials      576      575       -1     
Impacted Files Coverage Δ
ignite/cmd/plugin.go 42.43% <100.00%> (+0.65%) ⬆️
ignite/pkg/clictx/clictx.go 42.10% <100.00%> (+42.10%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@aljo242 aljo242 merged commit b1dcc17 into main Dec 5, 2022
@aljo242 aljo242 deleted the fix/interrupt-plugin-cmd branch December 5, 2022 16:36
aljo242 pushed a commit that referenced this pull request Dec 6, 2022
Due to the plugin architecture, the user wasn't able to interrupt a
plugin command via Ctrl+C. This is annoying if the plugin execution is
long.

By running the plugin execution in a goroutine and by listening to the
command context at the same time, we can fix that.

Co-authored-by: Alex Johnson <[email protected]>
aljo242 added a commit that referenced this pull request Dec 7, 2022
* vuln and math

* go-git

* go-yaml

* mockery + regen

* grpc

* chore(docs): docs v0.25.2 (latest) and nightly (#3206)

* chore(docs): docs v0.25.2 (latest) and nightly

* chore: add versionning documentation

* chore(ci): generate docs on new release

Co-authored-by: Alex Johnson <[email protected]>

* docs: "Hello, World!" tutorial rewrite (#3218)

* Getting started

* fix: dir name

* proto in cosmos sdk

* query wip

* finish the hello tutorial

* added a paragraph about default module

* chore: apply mdgofmt on docs

Had to re-apply the fixes I did on the docs to the `versioned_docs`
version.

Co-authored-by: Thomas Bruyelle <[email protected]>

* docs: fix path in hello tutorial (#3226)

* docs: fix path in hello tutorial

* formatting

* refactor(`config`):  organization (#3202)

* base

* base refactor

* refactor

* rename

* rename

* fix comment

* rename file

* format

* refactor base

* format

* refactor some imports

* imports refactor

* fix

* fix import

* refactor:

* changelog

* changelog

* Update ignite/services/network/networkchain/init.go

Co-authored-by: Jerónimo Albi <[email protected]>

* base -> baseconfig

* refactoring for clarity

* v12 -> v1

* fix tests

* fix integration

* fix name

* fix integration

* format

* Update ignite/cmd/cmd.go

Co-authored-by: Jerónimo Albi <[email protected]>

* imports

* LocateDefault logic

* lint fix

* refactor and test

* finish refactor

* revert

* move

* move

* rename

* fix tests

* fix test

* fix error statement for global plugins

* typo

* chainconfig

* imports

* fix imports

* address review

* testdata

* better import

* fix test

* fix

Co-authored-by: Jerónimo Albi <[email protected]>

* feat(CI): add codecov coverage (#3220)

* add codecov coverage

* create a new script file for the coverage test and put the coverage file into the .gitignore

Co-authored-by: Alex Johnson <[email protected]>

* feat(plugin): support plugin path with hash (#3217)

* feat(plugin): support plugin path with hash

Move the clone logic to xgit.Clone so it can be used for instance in the
`network chain publish` command. In the future it could also support
private repos.

* improve code format

* remove useless TODO

Co-authored-by: Alex Johnson <[email protected]>

* fix: interrupt plugin commands (#3222)

Due to the plugin architecture, the user wasn't able to interrupt a
plugin command via Ctrl+C. This is annoying if the plugin execution is
long.

By running the plugin execution in a goroutine and by listening to the
command context at the same time, we can fix that.

Co-authored-by: Alex Johnson <[email protected]>

* docs: added manual section to the hello tutorial (#3227)

* docs: added manual section to the hello tutorial

* fix: formatting

* fix: formatting

* ignite generate proto-go

* fix: remove grpc_ prefix

* add ignite chain serve

* fix: phrasing when new files are created

* Update docs/docs/guide/03-hello.md

Co-authored-by: Alex Johnson <[email protected]>

* Update docs/docs/guide/03-hello.md

Co-authored-by: Alex Johnson <[email protected]>

Co-authored-by: Alex Johnson <[email protected]>

* fix: removed grpc_* prefix from query files in scaffolded chain (#3224)

* Removed grpc_* prefix from query files in scaffolded chain

* modified:changelog.md

* modified:changelog.md

* Removed grpc_* prefix from query files in scaffolded chain

Co-authored-by: Alex Johnson <[email protected]>

* chore: go formatting (#3232)

Co-authored-by: aljo242 <[email protected]>

* docs(cli): update generated docs (#3207)

Co-authored-by: aljo242 <[email protected]>
Co-authored-by: Alex Johnson <[email protected]>

* chore(docs): Fix some broken links and add a link checker for the markdown files (#3219)

* Fix some broken links and add a link checker for the markdown files

* add md link checker config

* fix broken links

* remove redirect links from checker

* remove redirect links from checker

* remove duplicated config

* remove babylon blockchain link and change master to main for cosmos-sdk docs

* fix broken links

* remove sdk modules links because the redirection

* github moved links

* github moved links

* fix local links

* remove atom ide

* ignite guides

* add link checkr configs

* remove unused slash for urls

* exclude github 403 result links

* fix github link

Co-authored-by: Alex Johnson <[email protected]>

Co-authored-by: Albert Le Batteux <[email protected]>
Co-authored-by: Denis Fadeev <[email protected]>
Co-authored-by: Thomas Bruyelle <[email protected]>
Co-authored-by: Jerónimo Albi <[email protected]>
Co-authored-by: Danilo Pantani <[email protected]>
Co-authored-by: Thomas Bruyelle <[email protected]>
Co-authored-by: Saumya <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: aljo242 <[email protected]>
Jchicode pushed a commit to Jchicode/cli that referenced this pull request Aug 9, 2023
Due to the plugin architecture, the user wasn't able to interrupt a
plugin command via Ctrl+C. This is annoying if the plugin execution is
long.

By running the plugin execution in a goroutine and by listening to the
command context at the same time, we can fix that.

Co-authored-by: Alex Johnson <[email protected]>
Jchicode pushed a commit to Jchicode/cli that referenced this pull request Aug 9, 2023
* vuln and math

* go-git

* go-yaml

* mockery + regen

* grpc

* chore(docs): docs v0.25.2 (latest) and nightly (ignite#3206)

* chore(docs): docs v0.25.2 (latest) and nightly

* chore: add versionning documentation

* chore(ci): generate docs on new release

Co-authored-by: Alex Johnson <[email protected]>

* docs: "Hello, World!" tutorial rewrite (ignite#3218)

* Getting started

* fix: dir name

* proto in cosmos sdk

* query wip

* finish the hello tutorial

* added a paragraph about default module

* chore: apply mdgofmt on docs

Had to re-apply the fixes I did on the docs to the `versioned_docs`
version.

Co-authored-by: Thomas Bruyelle <[email protected]>

* docs: fix path in hello tutorial (ignite#3226)

* docs: fix path in hello tutorial

* formatting

* refactor(`config`):  organization (ignite#3202)

* base

* base refactor

* refactor

* rename

* rename

* fix comment

* rename file

* format

* refactor base

* format

* refactor some imports

* imports refactor

* fix

* fix import

* refactor:

* changelog

* changelog

* Update ignite/services/network/networkchain/init.go

Co-authored-by: Jerónimo Albi <[email protected]>

* base -> baseconfig

* refactoring for clarity

* v12 -> v1

* fix tests

* fix integration

* fix name

* fix integration

* format

* Update ignite/cmd/cmd.go

Co-authored-by: Jerónimo Albi <[email protected]>

* imports

* LocateDefault logic

* lint fix

* refactor and test

* finish refactor

* revert

* move

* move

* rename

* fix tests

* fix test

* fix error statement for global plugins

* typo

* chainconfig

* imports

* fix imports

* address review

* testdata

* better import

* fix test

* fix

Co-authored-by: Jerónimo Albi <[email protected]>

* feat(CI): add codecov coverage (ignite#3220)

* add codecov coverage

* create a new script file for the coverage test and put the coverage file into the .gitignore

Co-authored-by: Alex Johnson <[email protected]>

* feat(plugin): support plugin path with hash (ignite#3217)

* feat(plugin): support plugin path with hash

Move the clone logic to xgit.Clone so it can be used for instance in the
`network chain publish` command. In the future it could also support
private repos.

* improve code format

* remove useless TODO

Co-authored-by: Alex Johnson <[email protected]>

* fix: interrupt plugin commands (ignite#3222)

Due to the plugin architecture, the user wasn't able to interrupt a
plugin command via Ctrl+C. This is annoying if the plugin execution is
long.

By running the plugin execution in a goroutine and by listening to the
command context at the same time, we can fix that.

Co-authored-by: Alex Johnson <[email protected]>

* docs: added manual section to the hello tutorial (ignite#3227)

* docs: added manual section to the hello tutorial

* fix: formatting

* fix: formatting

* ignite generate proto-go

* fix: remove grpc_ prefix

* add ignite chain serve

* fix: phrasing when new files are created

* Update docs/docs/guide/03-hello.md

Co-authored-by: Alex Johnson <[email protected]>

* Update docs/docs/guide/03-hello.md

Co-authored-by: Alex Johnson <[email protected]>

Co-authored-by: Alex Johnson <[email protected]>

* fix: removed grpc_* prefix from query files in scaffolded chain (ignite#3224)

* Removed grpc_* prefix from query files in scaffolded chain

* modified:changelog.md

* modified:changelog.md

* Removed grpc_* prefix from query files in scaffolded chain

Co-authored-by: Alex Johnson <[email protected]>

* chore: go formatting (ignite#3232)

Co-authored-by: aljo242 <[email protected]>

* docs(cli): update generated docs (ignite#3207)

Co-authored-by: aljo242 <[email protected]>
Co-authored-by: Alex Johnson <[email protected]>

* chore(docs): Fix some broken links and add a link checker for the markdown files (ignite#3219)

* Fix some broken links and add a link checker for the markdown files

* add md link checker config

* fix broken links

* remove redirect links from checker

* remove redirect links from checker

* remove duplicated config

* remove babylon blockchain link and change master to main for cosmos-sdk docs

* fix broken links

* remove sdk modules links because the redirection

* github moved links

* github moved links

* fix local links

* remove atom ide

* ignite guides

* add link checkr configs

* remove unused slash for urls

* exclude github 403 result links

* fix github link

Co-authored-by: Alex Johnson <[email protected]>

Co-authored-by: Albert Le Batteux <[email protected]>
Co-authored-by: Denis Fadeev <[email protected]>
Co-authored-by: Thomas Bruyelle <[email protected]>
Co-authored-by: Jerónimo Albi <[email protected]>
Co-authored-by: Danilo Pantani <[email protected]>
Co-authored-by: Thomas Bruyelle <[email protected]>
Co-authored-by: Saumya <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: aljo242 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-changelog Don't check changelog for new entries
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants