refactor: add&use chaincmdrunner package #612
Conversation
40fdbed to
755cc5a
Compare
755cc5a to
05bc75d
Compare
93f8b0b to
08dab30
Compare
08dab30 to
e7b3711
Compare
| chaincmd.WithChainID(id), | ||
| chaincmd.WithHome(c.Home()), | ||
| ) | ||
| chaincmd.WithKeyringBackend(chaincmd.KeyringBackendTest), |
There was a problem hiding this comment.
Doing this we assume that all chains will use the test keyring backend. Currently, this is not what we have we chain used through networkbuilder for example
There was a problem hiding this comment.
Hm, I just checked the source code of networkbuilder, are you sure that we don't use test keyring backend there? Because it seems we don't specify this while adding accounts etc., and SDK chains by default uses the test backend.
PS, as a feature, I think we should let this to be configurable by the user for the network commands.
There was a problem hiding this comment.
I think the os keyring backend is used by default.
This works on Gitpod so it's ok for me to let it, the value will become configurable in #615
There was a problem hiding this comment.
Hm, yes, that's right. Only, the apps scaffold by Starport have it pointed to test. Regardless, we should be explicitly setting a value either as test or os to introduce a fixed behavior. @fadeev what're your thoughts on witch value we should set by default for starport network chains?
There was a problem hiding this comment.
I think starport network chains should use os keyring by default, because these commands are meant to run in "production" environments.
| app App | ||
| cmd chaincmd.ChainCmd | ||
| app App | ||
| chain *Chain |
There was a problem hiding this comment.
The plugin is already an attribute of the chain so I don't think it's a good pattern to have.
But it's ok to let it now. In #615, I do a refactoring and let chain providing the command instance when calling functions from the plugin
There was a problem hiding this comment.
Yes, this is a circular dependency right there. 😈
I was planning to get rid of it and in-fact improve the overall plugin architecture within #608. Let's coordinate on this to not get overlapped with each other's code. ⌨️ 💻
… develop * 'develop' of https://github.com/tendermint/starport: refactor: add&use chaincmdrunner package (#612)
`chaincmdrunner` provides a high level API to execute commands of an SDK app through programmatic access. * refactor `services/` to use this new pkg. * init cmdchainrunner pkg * start using chaincmdrunner in chain and networkbuilder services * cleanup * fix linter * cosmetics
chaincmdrunnerprovides a high level API to execute commands of an SDK app through programaticall access.services/to use this new pkg.