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 priority of client url and VAULT_ADDR environment variable #423

Merged
merged 2 commits into from Apr 4, 2019

Conversation

Projects
None yet
3 participants
@andytumelty
Copy link
Contributor

commented Apr 3, 2019

Fixes #421

The behaviour of the vault CLI client is:

  • if no -address cli parameter is set, try to use VAULT_ADDR environment variable
  • if both -address and VAULT_ADDR are set, use the -address passed in the vault cli command.

This PR updates the hvac client with the same precedence (url client parameter > VAULT_ADDR environment variable > default client URL).

Fix precedence of `VAULT_ADDR` environment variable vs client `url` p…
…arameter

Fixes #421.

The vault CLI priorities the `-address` flag over the `VAULT_ADDR`
environment variable. This commit aligns hvac to that behaviour.
@mrsiesta

This comment has been minimized.

Copy link

commented on ed85a27 Apr 3, 2019

This looks good to me if you'd like to put up the PR for this :)

Edit: Actually this looks mostly good to me, my only suggestion is to move the default URL into the constants/client.py file and import the value from there.

@mrsiesta
Copy link
Contributor

left a comment

Looks good to me, thanks!

@jeffwecan

This comment has been minimized.

Copy link
Collaborator

commented Apr 4, 2019

Thanks @andytumelty! I will try to get this into a patch release in the next day or so 👍

@jeffwecan jeffwecan added this to the 0.8.2 milestone Apr 4, 2019

@jeffwecan jeffwecan merged commit 2556c65 into hvac:develop Apr 4, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jeffwecan jeffwecan referenced this pull request Apr 4, 2019

Merged

Release v0.8.2 #424

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.