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

Allow :env to be configurable from env variables #80

Merged
merged 2 commits into from
Jun 28, 2017

Conversation

zorbash
Copy link
Contributor

@zorbash zorbash commented Jun 27, 2017

In your config/config.exs you can set {:system, "ENV_VARIABLE_HERE"} to allow the
env to be set with environmental variables supplied on runtime.

config :elixometer, env: {:system, "ELIXOMETER_ENV"}

@jparise jparise requested a review from scohen June 27, 2017 14:57
@zorbash zorbash force-pushed the configurable-env branch from f2a56c2 to 645ea63 Compare June 27, 2017 16:27
@jparise
Copy link
Collaborator

jparise commented Jun 27, 2017

@zorbash, any idea what's going on with those test failures?

@zorbash zorbash force-pushed the configurable-env branch from 645ea63 to 676ff65 Compare June 27, 2017 22:42
original_env = Application.get_env(:elixometer, :env)
Application.put_env(:elixometer, :env, {:system, "ELIXOMETER_MISSING_ENV"})


Choose a reason for hiding this comment

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

There should be no more than 1 consecutive blank lines.

@zorbash zorbash force-pushed the configurable-env branch from 676ff65 to 4699850 Compare June 27, 2017 22:44
@zorbash
Copy link
Contributor Author

zorbash commented Jun 27, 2017

@jparise

I had to add:

to ensure cleanup even when an assertion fails

and


assert get_metric_value("elixometer.spirals.precomputed_counter", :one) == {:ok, 123}

Application.put_env(:elixometer, :env, original_env)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you need to do this in both the tests and in the on_exit callback?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since it's in the on_exit callback it's no longer needed in the tests. Removing ✂️ .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed.

In your config/config.exs you can set {:system, "ENV_VARIABLE"} to allow
the env to be set with environmental variables supplied on runtime.

```
config :elixometer, env: {:system, "ELIXOMETER_ENV"}
```
@zorbash zorbash force-pushed the configurable-env branch from 4699850 to a6ea1c9 Compare June 28, 2017 08:46
@jparise
Copy link
Collaborator

jparise commented Jun 28, 2017

Looks good, @zorbash! Could you also mention this in the README.md's "Configuration" section? Once that's done, I'll merge this.

@zorbash zorbash force-pushed the configurable-env branch from 47814b9 to 973227b Compare June 28, 2017 15:47
@zorbash
Copy link
Contributor Author

zorbash commented Jun 28, 2017

@jparise I added a mention.

@jparise jparise merged commit ef09047 into pinterest:master Jun 28, 2017
@jparise
Copy link
Collaborator

jparise commented Jun 28, 2017

Thanks!

@zorbash zorbash deleted the configurable-env branch June 28, 2017 16:34
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.

3 participants