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

feat: add bidirectional communication to plugin system #3544

Conversation

jeronimoalbi
Copy link
Member

@jeronimoalbi jeronimoalbi commented Jun 13, 2023

Requires #3529

Bidirectional communication allows plugins to make calls to the CLI which is useful for example to be able to access code analysis features that are implemented by the CLI as Go packages.

@jeronimoalbi jeronimoalbi added type:feat To implement new feature. component:extensions Related to Ignite Extensions. labels Jun 13, 2023
@jeronimoalbi jeronimoalbi requested a review from clockworkgr June 13, 2023 11:39
@jeronimoalbi jeronimoalbi self-assigned this Jun 13, 2023
@jeronimoalbi jeronimoalbi changed the title feat: add bidirectional communication support to plugin system feat: add bidirectional communication to plugin system Jun 13, 2023
@jeronimoalbi
Copy link
Member Author

The changes in this PR add a new argument called "Analizer" to the plugin interface methods Execute, ExecuteHookPre, ExecuteHookPost and ExecuteHookCleanUp. This argument allows the CLI instance to expose features that plugins can call to get more detailed information about a blockchain app, like dependencies or proto file information which can be extracted using the existing CLI packages.

This feature is specially important for plugins implemented in languages other than Go which could benefit from the analysis code that is already implemented in Go by the CLI.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 15, 2023

Visit the preview URL for this PR (updated for commit 2da6db5):

https://igntservices-docs--pr3544-feat-plugin-system-i-onfhim44.web.app

(expires Wed, 25 Oct 2023 13:23:25 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 95379efd94dd497aaa37c2d0354e6e2cafca5ec5

@clockworkgr
Copy link
Collaborator

Looks great!

I think we should move away from analysis/analyzer as not all future API calls might have to do with analysis.

Maybe something as simple as PluginAPI ? Which is exactly what it is?

@jeronimoalbi
Copy link
Member Author

jeronimoalbi commented Jun 16, 2023

Looks great!

I think we should move away from analysis/analyzer as not all future API calls might have to do with analysis.

Maybe something as simple as PluginAPI ? Which is exactly what it is?

Yes, "Analyzer" is definitely not a good name, it was based on the initial idea we want to implement but it won't be a good name moving forward.

The PluginAPI name sounds good to me 👍

Alternatives could also be:

  • IgniteAPI
  • ClientAPI, the API is exposed by the plugin client which is Ignite CLI

@github-actions github-actions bot added the type:services Service-related issues. label Aug 8, 2023
Pantani and others added 3 commits August 10, 2023 13:31
…lugin-system-improvements-versioning-analysis
…lugin-system-improvements-versioning-analysis
* refactor: Analyzer/analizer -> ClientAPI

* refactor: rename proto files and rebuild

* refactor: Add json tags

* wip/refactor: Module analysis

* feat: Add chain reference to plugin ClientAPI

* feat: Complete Dependencies ClientAPI method

* fix: Address review comments

* feat: Remove services/chain dep from pkg/cosmosanalysis as per discussion

* wip: remove deptools install

* feat: package-specific includes

* fix: Replace Module List call with Chain Info call

* chore: Remove chain analysis code

* feat: ChainInfo API example template

* chore: Update template cli reference

* chore: clean up PR

* fix: Address review comments

* fix: address review comments

* fix: address review comments

* fix: Tests and linting

* chore: add changelog

* fix: linting issues

* tests: fix issue with client api in plugin tests

* tests: fix plugin template for integration tests

---------

Co-authored-by: jeronimoalbi <[email protected]>
@jeronimoalbi
Copy link
Member Author

The changes in this PR add a new argument called "Analizer" to the plugin interface methods Execute, ExecuteHookPre, ExecuteHookPost and ExecuteHookCleanUp. This argument allows the CLI instance to expose features that plugins can call to get more detailed information about a blockchain app, like dependencies or proto file information which can be extracted using the existing CLI packages.

The argument was renamed to "ClientAPI"

@jeronimoalbi jeronimoalbi marked this pull request as ready for review September 5, 2023 15:53
…lugin-system-improvements-versioning-analysis
Copy link
Member

@julienrbrt julienrbrt left a comment

Choose a reason for hiding this comment

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

Changes are straightforward! Nice work!

proto/ignite/services/plugin/grpc/v1/service.proto Outdated Show resolved Hide resolved
@jeronimoalbi jeronimoalbi merged commit bf692a5 into feat/plugin-system-improvements-versioning Oct 19, 2023
29 checks passed
@jeronimoalbi jeronimoalbi deleted the feat/plugin-system-improvements-versioning-analysis branch October 19, 2023 13:56
Pantani added a commit that referenced this pull request Nov 10, 2023
* chore: update plugin template Go version to 1.20

* docs: add documentation to clarify the plugin cache purpose

* chore: improve plugins code spacing for better redability

* chore: add context support to plugin scaffold

* refactor: move plugin RPC implementation to an `rpc.go` file

* refactor: simplify RPC plugin implementation

* feat: add proto buffer files for the gRPC plugins

* chore: generate plugin files from proto

* chore: remove extra blank line

* chore: add Flags to ExecutedCommand proto definition

* refactor: remove rpc plugin implementation

* refactor: move interface types methods to the generated types

* refactor: move plugin interface into grpc package

* feat: add gRPC plugin implementation

* refactor: change plugin to use gRPC for communication

* refactor: change plugin command to use the new gRPC interface

* refactor: add context to gRPC plugin client communication

* refactor: move plugin interface and gRPC implementation

Done to have a better package API.

* chore: add plugin flag type aliases for v1

* refactor: add flag contructor methods to executed command

* refactor: update plugin template to use gRPC

* test: fixed tests

* chore: move proto flag type enum definition into flag message

* chore: add changelog entry

* chore: correct flag type reference in plugin service tests

* test: improve plugin scaffold test

* test: change flag types

* chore: lower plugin template Go version to 1.19

This is temporary until CI/CD allows Go v1.20

* chore: use `hplugin` alias go hashicorp's go-plugin package

To keep consistency between imports in the plugin package.

* test: fix issue that killed plugins when testing

* test: fix plugin command tests

* fix: correct issue in plugin load

* chore: update plugin documentation

* chore: fix changelog

* chore: add buf to tool dependencies

* feat: add make file targets to generate code from proto

* fix: correct proto file linting issue

* chore: proto file format fix

* ci: add GitHub workflow to check and lint proto files

* ci: disable proto breaking change

It must be disabled until proto files are available in the main branch.

* fix: correct Buf repository name

* test: change plugin integration test to use a local example folder

This was done to allow CI to pass the test without the requirement of
merging the changes into the example plugin repository.

* chore: update Go and CLI versions

Co-authored-by: Danilo Pantani <[email protected]>

* ci: update GitHub workflows to use Go version 1.20

* fix: beam me up Scotty

* test: move command and manifest test to the right location

* test: add plugin gRPC types tests

* ci: correct CI issue with naked return

* fix: correct plugin command path issue

* chore: remove deprecated replace from Go mod

Co-authored-by: Julien Robert <[email protected]>

* chore: change Ignite App handshake config

Co-authored-by: Danilo Pantani <[email protected]>

* feat: add bidirectional communication to plugin system (#3544)

* chore: rename `types.proto`to `interface.proto`

* feat: add code analizer support

Adds initial analizer support to allow bidirectional communication
between the CLI and the plugins.

The analizer features should be defined and implemented on top of these
changes.

* feat: add analizer support to plugin's interface

* chore: add proto generated code

* chore: generate interface mocks

* refactor: change protocol implementation to support analizer

* chore: fix issue with missing return

* refactor: change plugin command to add analizer to calls

* feat: add proto types required for the cosmos/proto analysis packages

These types allows the implementation of the dependencies analyzer.

* chore: update plugin template to include analizer arguments

* fix: correct type in Analyzer name

* chore: update plugin documentation

* chore: rename analyzer file name because of typo

* fix: correct typo for analyzer names

* chore: update changelog

* test: fix plugin integration test

* test: fix plugin cmd unit tests

* test: fix plugin service unit test

* ci: fix unused argument warning

* refactor: Add basic GetChainInfo method to plugin API (#3561)

* refactor: Analyzer/analizer -> ClientAPI

* refactor: rename proto files and rebuild

* refactor: Add json tags

* wip/refactor: Module analysis

* feat: Add chain reference to plugin ClientAPI

* feat: Complete Dependencies ClientAPI method

* fix: Address review comments

* feat: Remove services/chain dep from pkg/cosmosanalysis as per discussion

* wip: remove deptools install

* feat: package-specific includes

* fix: Replace Module List call with Chain Info call

* chore: Remove chain analysis code

* feat: ChainInfo API example template

* chore: Update template cli reference

* chore: clean up PR

* fix: Address review comments

* fix: address review comments

* fix: address review comments

* fix: Tests and linting

* chore: add changelog

* fix: linting issues

* tests: fix issue with client api in plugin tests

* tests: fix plugin template for integration tests

---------

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

* chore: correct typos and simplify code

* chore: update pseudo version for plugin's `go.mod` template

* chore: fix typos

* fix: correct call to module dependency resolution

* chore: remove proto file comment

---------

Co-authored-by: Danilo Pantani <[email protected]>
Co-authored-by: Clockwork <[email protected]>

---------

Co-authored-by: Danilo Pantani <[email protected]>
Co-authored-by: Julien Robert <[email protected]>
Co-authored-by: Clockwork <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:ci CI/CD workflow and automated jobs. component:cmd component:configs component:extensions Related to Ignite Extensions. component:packages type:feat To implement new feature. type:services Service-related issues.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants