Skip to content

Use cobra and viper for configuration#175

Merged
benzcash merged 4 commits intomasterfrom
Use-cobra-and-viper-for-configuration
Mar 18, 2020
Merged

Use cobra and viper for configuration#175
benzcash merged 4 commits intomasterfrom
Use-cobra-and-viper-for-configuration

Conversation

@benzcash
Copy link
Contributor

This PR is designed to use the cobra and viper packages to provide additional configure options.

Changes

  • Added base main.go, so go build outputs lightwalletd
  • Moved the server startup to the "root" command
  • Create a subcommand for version
    Example usage:
./lightwalletd version
lightwalletd version v0.2.0
  • Tried to change the current flags as much as possible, but they are double-dashed now (we can add short options if desired)
Flags:
      --bind-addr string         the address to listen on (default "127.0.0.1:9067")
      --cache-size int           number of blocks to hold in the cache (default 80000)
      --config string            config file (default is current directory, lightwalletd.yaml)
  -h, --help                     help for lightwalletd
      --log-file string          log file to write to (default "./server.log")
      --log-level int            log level (logrus 1-7) (default 4)
      --no-tls-very-insecure     run without the required TLS certificate, only for debugging, DO NOT use in production
      --tls-cert string          the path to a TLS certificate (default "./cert.pem")
      --tls-key string           the path to a TLS key file (default "./cert.key")
      --zcash-conf-path string   conf file to pull RPC creds from (default "./zcash.conf")

Added functionality

Provide configuration from file

An example lightwalletd-example.yml is in the PR
Usage:

./lightwalletd --config ./lightwalletd-example.yml
Using config file: /home/bwilson/go/src/github.com/zcash-hackworks/lightwalletd/lightwalletd.yml

Or, rename it to just lightwalletd.yml and it will be auto-discovered

$ ./lightwalletd 
Using config file: /home/bwilson/go/src/github.com/zcash-hackworks/lightwalletd/lightwalletd.yml

Provide configuration from environment variables

The same options can be provided through environment variables
Example Usage:

TLS_CERT=/secrets/lightwallted/cert.pem \
TLS_KEY=/secrets/lightwallted/cert.key \
ZCASH_CONF_PATH=/srv/zcashd/zcash.conf \
CACHE_SIZE=10 \
LOG_FILE=/dev/stdout \
BIND_ADDR=0.0.0.0:9067 \
./lightwalletd

** NOTE ** We can preface these environmental variable with a string like LIGHTWALLETD_ or LWD_ to avoid collisions.

Copy link
Contributor

@defuse defuse left a comment

Choose a reason for hiding this comment

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

Nice improvement! Looks good modulo the one comment I left about visibly warning when there's no TLS.

Copy link
Collaborator

@LarryRuane LarryRuane left a comment

Choose a reason for hiding this comment

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

Very nice! When I run go build, (or make, same thing), a bunch of changes appear in source-controlled files go.mod and go.sum. Is that just me, or do those files also get modified for you? If so I think those changes should be part of this PR.

@benzcash
Copy link
Contributor Author

benzcash commented Mar 3, 2020

Is that just me, or do those files also get modified for you? If so I think those changes should be part of this PR.

Yeah, forgot them. Added!

@codecov-io
Copy link

codecov-io commented Mar 3, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@2687f7d). Click here to learn what that means.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #175   +/-   ##
=========================================
  Coverage          ?   32.54%           
=========================================
  Files             ?       11           
  Lines             ?     1395           
  Branches          ?        0           
=========================================
  Hits              ?      454           
  Misses            ?      884           
  Partials          ?       57
Impacted Files Coverage Δ
common/generatecerts.go 0% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2687f7d...1390b49. Read the comment docs.

@LarryRuane
Copy link
Collaborator

Should any changes be made to README.md, especially as relates to deployment, as part of this PR? Perhaps just what's in this PR's first comment.

@LarryRuane LarryRuane mentioned this pull request Mar 18, 2020
2 tasks
@LarryRuane LarryRuane force-pushed the Use-cobra-and-viper-for-configuration branch from 1390b49 to e71de4d Compare March 18, 2020 18:14
@benzcash benzcash merged commit 3b55f76 into master Mar 18, 2020
@benzcash benzcash deleted the Use-cobra-and-viper-for-configuration branch March 18, 2020 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants