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

[env] using Rails environment only when asked to do so #106

Merged
merged 3 commits into from
Apr 7, 2017

Conversation

ufoot
Copy link
Contributor

@ufoot ufoot commented Apr 7, 2017

Should fix the side effects described in #92

@ufoot ufoot added the bug Involves a bug label Apr 7, 2017
@ufoot ufoot requested review from palazzem and talwai April 7, 2017 05:38
@ufoot ufoot mentioned this pull request Apr 7, 2017
@ufoot ufoot added this to the 0.6.2 milestone Apr 7, 2017
Copy link
Contributor

@palazzem palazzem left a comment

Choose a reason for hiding this comment

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

Not totally sure about that. I still prefer removing the flag and set the env: nil by default if Rails.env can lead to issues. But if that's the preferable approach, that's good to be merged!

@@ -65,7 +65,8 @@ of the Datadog tracer, you can override the following defaults:
debug: false,
trace_agent_hostname: 'localhost',
trace_agent_port: 8126,
env: Rails.env,
use_rails_env: false,
Copy link
Contributor

@palazzem palazzem Apr 7, 2017

Choose a reason for hiding this comment

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

what I'm worried about, is that we're polluting a lot Rails settings to provide "almost any kind of behavior" with flags rather than be explicit (i.e.: if you want to use the Rails env, just set env: Rails.env)

@@ -88,7 +89,8 @@ Available settings are:
* ``debug``: set to true to enable debug logging.
* ``trace_agent_hostname``: set the hostname of the trace agent.
* ``trace_agent_port``: set the port the trace agent is listening on.
* ``env``: set the environment. Defaults to the Rails environment
* ``use_rails_env``: guess the Datadog environment from the Rails environment.
* ``env``: set the environment. Overridden by Rails environment if ``use_rails_env`` is set.
Copy link
Contributor

Choose a reason for hiding this comment

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

Not totally sure that is good to have two different settings to provide (and override) the same thing. If using the Rails.env could create issues, I'll probably go with a default env: nil and let the user to be explicit.

@ufoot ufoot merged commit cc08a18 into master Apr 7, 2017
@palazzem palazzem deleted the christian/railsdefaultenv branch April 7, 2017 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Involves a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants