Allow Tapioca commands to accept environment variables #1957
Conversation
paracycle
force-pushed
the
uk-passthrough-env-vars
branch
from
ebf1e69 to
58b981d
Compare
This change allows Tapioca commands to accept environment variables via the command line or the configuration file. This is useful for setting up the environment for the Tapioca commands, such as setting the custom environment variables that affect how an application is loaded.
paracycle
force-pushed
the
uk-passthrough-env-vars
branch
from
58b981d to
cc5011f
Compare
| sig { params(options: T.nilable(T::Hash[String, String])).void } | ||
| def set_all_environment_variables(options) # rubocop:disable Naming/AccessorMethodName | ||
| (options || {}).each do |key, value| | ||
| ENV[key] = value |
There was a problem hiding this comment.
So the --env values are overriding the ENV, is this really what we want?
If so, I think we should document this in the option description or the README.
There was a problem hiding this comment.
Also, if this is indeed what we want, is there a reason not to use ENV.merge!(options)?
There was a problem hiding this comment.
Yeah, I didn't think of ENV.merge!, that sounds better!
There was a problem hiding this comment.
So the
--envvalues are overriding theENV, is this really what we want?
What else could they be doing? Tapioca sets those env variables as the user has passed them and we don't care if they override some values or not. bin/tapioca dsl --env FOO:42 is the same thing that happens when someone runs FOO=42 bin/tapioca dsl, in either case we override whatever the value of the FOO env variable might have been to make it 42 for the duration of that run.
| $stderr.puts "ENVIRONMENT: \#{ENV.to_h.inspect}" | ||
| RB | ||
|
|
||
| result = @project.tapioca("dsl --env Foo:Bar --env Baz:1") |
There was a problem hiding this comment.
Can we add a test showing the behaviour of overriding system env?
| @@ -88,6 +88,7 @@ Commands: | |||
| Options: | |||
| -c, [--config=<config file path>] # Path to the Tapioca configuration file | |||
| # Default: sorbet/tapioca/config.yml | |||
| [--env=key:value] # Environment variables to set before running Tapioca | |||
There was a problem hiding this comment.
Nitpick: it might be my lack of familiarity with Thor, but it wasn't immediately obvious to me how we'd be passing multiple environment variables to the same command (I understood after reading the tests).
Could we add an example somewhere in the README of how to pass these?
There was a problem hiding this comment.
So, we already have a few hash type options and how they work is documented here: https://github.com/rails/thor/wiki/Method-Options#types-for-method_options
The main way is to use the --opt=key:value key2:value2 way, but if you mark the option as repeatable: true, then you can also do --opt=key:value --opt=key2:value2 too.
I guess I can add a section for how to pass env variables via command line or config file to make that more clear and link to the Thor docs for details.
| sig { params(options: T.nilable(T::Hash[String, String])).void } | ||
| def set_all_environment_variables(options) # rubocop:disable Naming/AccessorMethodName | ||
| (options || {}).each do |key, value| | ||
| ENV[key] = value |
There was a problem hiding this comment.
Also, if this is indeed what we want, is there a reason not to use ENV.merge!(options)?
This change allows Tapioca commands to accept environment variables via the command line or the configuration file. This is useful for setting up the environment for the Tapioca commands, such as setting the custom environment variables that affect how an application is loaded.
Motivation
#1952 (comment)
Implementation
Add a
class_option :envthat allows all commands to accept environment variables.Tests
Added a DSL CLI test