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: use client/v2 v2.0.0-beta3; fix broken client.toml usage #3210

Merged
merged 4 commits into from
Jul 22, 2024

Conversation

MSalopek
Copy link
Contributor

@MSalopek MSalopek commented Jul 21, 2024

Fixes a bug discovered during testing.

The CLI would ignore the variables from client.toml which forced operators to always use --node <IP:port> command line options.

The fix consists of changing root.go cmd wiring and updating cosmossdk.io/client/v2 v2.0.0-beta3. The application does not use depinject.

Related:

@MSalopek MSalopek added the scope: CLI Issues and features pertaining to Gaia's CLI client. label Jul 21, 2024
@p-offtermatt
Copy link
Contributor

Is there any automated testing we have that would help us confirm whether this fixes the issue, or do we have to check manually?

@MSalopek
Copy link
Contributor Author

MSalopek commented Jul 22, 2024

Is there any automated testing we have that would help us confirm whether this fixes the issue, or do we have to check manually?

Tests are with Hypha, we're migrating parts of it here.
Re-wiring our e2e tests it not feasible at this point since new features are already in the pipeline.

Easiest way to test is to spin up a local gaia with non-default API port (!= 26657), update the client.toml and try making queries without specifying --node.

E.g.

// client.toml
node = "tcp://localhost:26658"

Before the fix:

gaiad status
// fails with node not available on 127.0.0.1:26657 [the default!] -- node is actually on 26658

gaiad status --node tcp://127.0.0.1:26658
// passes

After the fix:

gaiad status
// passes because `node = "tcp://localhost:26658"` from config.toml gets applied

Copy link
Contributor

@dusan-maksimovic dusan-maksimovic left a comment

Choose a reason for hiding this comment

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

Administrative approval

Copy link
Contributor

@p-offtermatt p-offtermatt left a comment

Choose a reason for hiding this comment

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

Thanks for the answer. That makes sense to me, so LGTM

@MSalopek MSalopek merged commit 27da5e8 into main Jul 22, 2024
17 checks passed
@MSalopek MSalopek deleted the fix/update-root-cmd-v50 branch July 22, 2024 08:51
mergify bot pushed a commit that referenced this pull request Jul 22, 2024
* fix: use client/v2 v2.0.0-beta3; fix broken client.toml usage

* Update root.go

(cherry picked from commit 27da5e8)
MSalopek added a commit that referenced this pull request Jul 22, 2024
…#3224)

* fix: use client/v2 v2.0.0-beta3; fix broken client.toml usage

* Update root.go

(cherry picked from commit 27da5e8)

Co-authored-by: MSalopek <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:backport/v21.x scope: CLI Issues and features pertaining to Gaia's CLI client.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants