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

add back cli config option for flags #8529

Closed
4 tasks
okwme opened this issue Feb 5, 2021 · 14 comments · Fixed by #8953
Closed
4 tasks

add back cli config option for flags #8529

okwme opened this issue Feb 5, 2021 · 14 comments · Fixed by #8953
Assignees
Labels
C:CLI T: Dev UX UX for SDK developers (i.e. how to call our code)
Milestone

Comments

@okwme
Copy link
Contributor

okwme commented Feb 5, 2021

Summary

I noticed you can't use a command like gaiacli config keyring-backend test anymore.
It makes commands really cumbersome when you have to add a flag like that onto everything, is there no way to configure the command anymore?

Problem Definition

Proposal


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@okwme okwme added the T: Dev UX UX for SDK developers (i.e. how to call our code) label Feb 5, 2021
@migueldingli1997
Copy link
Contributor

I also miss this command! Especially since I like to set up scripts with sometimes long sequences of transactions. Without the config, these sort of scripts will be less readable due to longer lines (Even though for this specific purpose I can set up functions which abstract away these flags)

@alessio
Copy link
Contributor

alessio commented Feb 24, 2021

It was a hack-ish, manually maintained tiny thing that I wrote originally long time ago. I was never aware that it was so much used.
We need to bring it back.

CC'ing @jgimeno @sahith-narahari @fdymylja

@fdymylja
Copy link
Contributor

If we could understand better the scope of this. What is the reason? Is it to support multiple chain configs from the same daemon?

@alessio
Copy link
Contributor

alessio commented Feb 24, 2021

If we could understand better the scope of this. What is the reason? Is it to support multiple chain configs from the same daemon?

Nope. Cosmos SDK applications used to read client configuration from an INI file. Such file could be edited either manually or via the command appcli config KEY VALUE, e.g. gaiacli config keyring-backend file. In a way, it kinda resembled git config's behaviour.

@clevinson
Copy link
Contributor

@alessio is someone from your team addressing this or should we assign someone from the regen side?

@alessio
Copy link
Contributor

alessio commented Feb 26, 2021

Do you want to include this in v0.42? If the answer to the latter is yes, then yes please feel free to reassign to someone at Regen as we are at capacity (plus, our backlog is growing).

@amaury1093
Copy link
Contributor

I'm not sure if it's in scope of this issue, but a refactor of the whole config situation would be cool: #8132 (comment), and might make this task easier.

@alessio
Copy link
Contributor

alessio commented Mar 1, 2021

Yes, I agree 👍

@alessio
Copy link
Contributor

alessio commented Mar 1, 2021

C'ing @gsora

@clevinson
Copy link
Contributor

@cyberbono3 who has just joined the regen team will be taking a look at this from our side :)

@aaronc
Copy link
Member

aaronc commented Mar 3, 2021

One of the reasons for removing this is dependency on a global viper object. We will need to figure out how to do this cleanly without using globals. Maybe a scoped viper instance or something totally outside the viper stack.

@amaury1093
Copy link
Contributor

amaury1093 commented Mar 4, 2021

Just had a talk with @cyberbono3 on this. I believe people want this on the hub right now, so i propose 2 steps:

  • Step 1 (will be backported for the hub). Add the config subcommand as described in the 1st post, which is non-breaking. I'm proposing to create a new $NODE_DIR/config/client.toml file to hold all client-side configuration (e.g. keyring-backend, node, chain-id etc...). Basically what @alessio described in add back cli config option for flags #8529 (comment).

    • note: this won't pollute app.toml, which is more server/node configuration.
  • Step 2 (for 0.43+). Think about refactoring config as a whole, with the objectives in mind to:

    • remove dependency on singletons
    • make sure we have the exact same configurable variables via flags, ENV variables and config file, and define a priority between them when multiple are set

Any feedback on this?

@alessio
Copy link
Contributor

alessio commented Mar 4, 2021

Just had a talk with @cyberbono3 on this. I believe people want this on the hub right now, so i propose 2 steps:

  • Step 1 (will be backported for the hub). Add the config subcommand as described in the 1st post, which is non-breaking. I'm proposing to create a new $NODE_DIR/config/client.toml file to hold all client-side configuration (e.g. keyring-backend, node, chain-id etc...). Basically what @alessio described in #8529 (comment).

    • note: this won't pollute app.toml, which is more server/node configuration.
  • Step 2 (for 0.43+). Think about refactoring config as a whole, with the objectives in mind to:

    • remove dependency on singletons
    • make sure we have the exact same configurable variables via flags, ENV variables and config file, and define a priority between them when multiple are set

Any feedback on this?

Sounds like we have a plan!

@amaury1093
Copy link
Contributor

Step 2 (for 0.43+). Think about refactoring config as a whole, with the objectives in mind to:

This is tracked in #9034

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:CLI T: Dev UX UX for SDK developers (i.e. how to call our code)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants