Add env variable to cmd flags #9040

cyberbono3 · 2021-04-01T03:45:16Z

Description

Added v.AutomaticEnv() to Viper field of client.Context.
Implemented various test cases to test priority rules flag > env var > config for client config.

closes: #8179

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

Targeted PR against correct branch (see CONTRIBUTING.md)
Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
Code follows the module structure standards.
Wrote unit and integration tests
Updated relevant documentation (docs/) or specification (x/<module>/spec/)
Added relevant godoc comments.
Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
Re-reviewed Files changed in the Github PR explorer
Review Codecov Report in the comment section below once CI passes

client/config/cmd_test.go

cyberbono3 · 2021-04-06T05:31:36Z

@AmauryM Could you leave your feedback, please?

client/config/cmd_test.go

client/config/config_test.go

client/config/cmd_test.go

client/config/config_test.go

codecov · 2021-04-07T21:15:31Z

Codecov Report

Merging #9040 (8230297) into master (cf7b03e) will decrease coverage by 0.03%.
The diff coverage is 33.33%.

@@            Coverage Diff             @@
##           master    #9040      +/-   ##
==========================================
- Coverage   58.83%   58.79%   -0.04%     
==========================================
  Files         580      583       +3     
  Lines       32648    32750     +102     
==========================================
+ Hits        19208    19255      +47     
- Misses      11174    11218      +44     
- Partials     2266     2277      +11

Impacted Files	Coverage Δ
client/context.go	`54.86% <0.00%> (-0.99%)`	⬇️
simapp/simd/cmd/root.go	`61.71% <100.00%> (ø)`
client/config/toml.go	`55.55% <0.00%> (ø)`
client/config/config.go	`53.33% <0.00%> (ø)`
client/config/cmd.go	`40.38% <0.00%> (ø)`

amaury1093

Nice!

amaury1093

actually, before auto-merging, let me just test manually on the CLI too

client/context.go

amaury1093

tACK.

i tested:

# setting manually WithViper("ABC")
ABC_NODE=localhost:1 simd q staking validators

simapp/simd/cmd/root.go

amaury1093 · 2021-04-12T09:41:09Z

@cyberbono3 Could you fix the lint errors?

Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com>

cyberbono3 · 2021-04-12T21:49:44Z

@cyberbono3 Could you fix the lint errors?

Sure. I have fixed some of them.

However, I still face some Issues:

I have no idea why TestPaginatedVotesQuery fails.
This no file shown here that has to be gofmted.

amaury1093 · 2021-04-13T10:32:26Z

I have no idea why TestPaginatedVotesQuery fails.

It's flaky, see #9010

This no file shown here that has to be gofmted.

See https://github.com/cosmos/cosmos-sdk/pull/9040/files#diff-146f73b0327c426deb3126e44f9234c7a6fe190c1bcb4c98ab8366b3485922edR116

client/config/config_test.go

amaury1093 · 2021-05-12T11:49:31Z

@mergify io backport release/v0.42.x

mergify · 2021-05-12T11:49:46Z

Sorry but I didn't understand the command. Please consult the commands documentation 📚.

Hey, I reacted but my real name is @Mergifyio

amaury1093 · 2021-05-12T11:50:08Z

@Mergifyio backport release/v0.42.x

* first draft * add tests in config_test package * refactored, all tests pass, r4r * ready for review * add envPrefix * Update simapp/simd/cmd/root.go Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> * fix linter issues * minor fixes Co-authored-by: Alessio Treglia <alessio@tendermint.com> Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> (cherry picked from commit a465ae1) # Conflicts: # client/context.go # simapp/simd/cmd/root.go

mergify · 2021-05-12T11:50:59Z

Command backport release/v0.42.x: success

Backports have been created

#9318 Add env variable to cmd flags (backport #9040) has been created for branch release/v0.42.x

* Add env variable to cmd flags (#9040) * first draft * add tests in config_test package * refactored, all tests pass, r4r * ready for review * add envPrefix * Update simapp/simd/cmd/root.go Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> * fix linter issues * minor fixes Co-authored-by: Alessio Treglia <alessio@tendermint.com> Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> (cherry picked from commit a465ae1) # Conflicts: # client/context.go # simapp/simd/cmd/root.go * Remove line Co-authored-by: Andrei Ivasko <cyberbono3@gmail.com> Co-authored-by: Amaury M <1293565+amaurym@users.noreply.github.com> Co-authored-by: Alessio Treglia <alessio@tendermint.com>

* first draft * add tests in config_test package * refactored, all tests pass, r4r * ready for review * add envPrefix * Update simapp/simd/cmd/root.go Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> * fix linter issues * minor fixes Co-authored-by: Alessio Treglia <alessio@tendermint.com> Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

first draft

ba1f732

cyberbono3 commented Apr 1, 2021

View reviewed changes

client/config/cmd_test.go Outdated Show resolved Hide resolved

cyberbono3 requested a review from amaury1093 April 1, 2021 03:49

amaury1093 reviewed Apr 1, 2021

View reviewed changes

add tests in config_test package

b471bcd

amaury1093 reviewed Apr 6, 2021

View reviewed changes

client/config/cmd_test.go Outdated Show resolved Hide resolved

client/config/config_test.go Outdated Show resolved Hide resolved

refactored, all tests pass, r4r

3434789

cyberbono3 requested a review from amaury1093 April 7, 2021 01:26

amaury1093 reviewed Apr 7, 2021

View reviewed changes

client/config/config_test.go Show resolved Hide resolved

client/config/config_test.go Outdated Show resolved Hide resolved

client/config/cmd_test.go Outdated Show resolved Hide resolved

client/config/config_test.go Outdated Show resolved Hide resolved

cyberbono3 requested a review from alessio April 7, 2021 18:02

cyberbono3 marked this pull request as ready for review April 7, 2021 18:04

cyberbono3 requested review from aaronc and alexanderbez as code owners April 7, 2021 18:04

cyberbono3 self-assigned this Apr 7, 2021

cyberbono3 force-pushed the cyberbono3/8179-add-env-vars branch from 30333f2 to 3434789 Compare April 7, 2021 21:00

ready for review

a6033b7

cyberbono3 requested a review from amaury1093 April 7, 2021 21:04

alessio approved these changes Apr 8, 2021

View reviewed changes

Merge branch 'master' into cyberbono3/8179-add-env-vars

5fa7aed

amaury1093 added A:automerge Automatically merge PR once all prerequisites pass. backport/0.42.x (Stargate) labels Apr 8, 2021

amaury1093 approved these changes Apr 8, 2021

View reviewed changes

amaury1093 removed the A:automerge Automatically merge PR once all prerequisites pass. label Apr 8, 2021

amaury1093 suggested changes Apr 8, 2021

View reviewed changes

client/context.go Outdated Show resolved Hide resolved

cyberbono3 and others added 2 commits April 8, 2021 08:27

add envPrefix

777f38b

Merge branch 'master' into cyberbono3/8179-add-env-vars

23ede7d

amaury1093 approved these changes Apr 12, 2021

View reviewed changes

simapp/simd/cmd/root.go Outdated Show resolved Hide resolved

amaury1093 and others added 3 commits April 12, 2021 11:41

Merge branch 'master' into cyberbono3/8179-add-env-vars

3f74c78

Update simapp/simd/cmd/root.go

b97460c

Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com>

fix linter issues

c953f78

cyberbono3 force-pushed the cyberbono3/8179-add-env-vars branch from ac00a44 to c953f78 Compare April 12, 2021 21:23

amaury1093 reviewed Apr 13, 2021

View reviewed changes

client/config/config_test.go Outdated Show resolved Hide resolved

minor fixes

01fab9b

amaury1093 added the A:automerge Automatically merge PR once all prerequisites pass. label Apr 13, 2021

Merge branch 'master' into cyberbono3/8179-add-env-vars

8230297

mergify bot merged commit a465ae1 into master Apr 13, 2021

mergify bot deleted the cyberbono3/8179-add-env-vars branch April 13, 2021 17:32

mergify bot mentioned this pull request May 12, 2021

Add env variable to cmd flags (backport #9040) #9318

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add env variable to cmd flags #9040

Add env variable to cmd flags #9040

cyberbono3 commented Apr 1, 2021 •

edited

Loading

cyberbono3 commented Apr 6, 2021 •

edited

Loading

codecov bot commented Apr 7, 2021 •

edited

Loading

amaury1093 left a comment

amaury1093 left a comment

amaury1093 left a comment

amaury1093 commented Apr 12, 2021

cyberbono3 commented Apr 12, 2021 •

edited

Loading

amaury1093 commented Apr 13, 2021

amaury1093 commented May 12, 2021

mergify bot commented May 12, 2021

amaury1093 commented May 12, 2021

mergify bot commented May 12, 2021

Add env variable to cmd flags #9040

Add env variable to cmd flags #9040

Conversation

cyberbono3 commented Apr 1, 2021 • edited Loading

Description

cyberbono3 commented Apr 6, 2021 • edited Loading

codecov bot commented Apr 7, 2021 • edited Loading

Codecov Report

amaury1093 left a comment

Choose a reason for hiding this comment

amaury1093 left a comment

Choose a reason for hiding this comment

amaury1093 left a comment

Choose a reason for hiding this comment

amaury1093 commented Apr 12, 2021

cyberbono3 commented Apr 12, 2021 • edited Loading

amaury1093 commented Apr 13, 2021

amaury1093 commented May 12, 2021

mergify bot commented May 12, 2021

amaury1093 commented May 12, 2021

mergify bot commented May 12, 2021

cyberbono3 commented Apr 1, 2021 •

edited

Loading

cyberbono3 commented Apr 6, 2021 •

edited

Loading

codecov bot commented Apr 7, 2021 •

edited

Loading

cyberbono3 commented Apr 12, 2021 •

edited

Loading