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

Change behavior of flag arguments specified as env vars #2539

Closed
2 tasks done
jacobsvante opened this issue Jun 16, 2021 · 1 comment · Fixed by #2664
Closed
2 tasks done

Change behavior of flag arguments specified as env vars #2539

jacobsvante opened this issue Jun 16, 2021 · 1 comment · Fixed by #2664
Labels

Comments

@jacobsvante
Copy link

jacobsvante commented Jun 16, 2021

Please complete the following tasks

  • I have searched the discussions
  • I have searched the existing issues

Describe your use case

I have a little CLI app that I also execute as a CronJob in Kubernetes. For this I used Helm to set up templates and "values files" which in turn generate the required Kubernetes manifests.

The little Clap CLI I've cfeated is called like this:

./sync-data --store-source-cache

Now when I set up the Kubernetes CronJob values file I prefer to do it the Twelve-Factor App way, specifying the options and arguments as environment variables instead (which might differ between staging/production etc).

It works well for arguments and parameters. But for flags it feels a bit awkward, having to comment out the environment variable from the values.yaml files (there's one with base settings, one for prod env, one for staging env).

So currently it looks like this:

# base/values.yaml (base file)
envVars:
  SOME_ARGUMENT: "SOME VALUE"
  # LOAD_SOURCE_CACHE: "true"

# prod/values.yaml
envVars:
  LOAD_SOURCE_CACHE: "true"

# staging/values.yaml
envVars: {}
  # LOAD_SOURCE_CACHE: "true"

As you can see I have to comment out LOAD_SOURCE_CACHE: "" in the base and staging values files. (The base and the prod/staging file are merged together)

Example main.rs which shows current behavior:

use clap::Clap;

#[derive(Clap)]
struct Opts {
    #[clap(long, env, takes_value = false)]
    load_source_cache: bool,
}

fn main() {
    let opts: Opts = Opts::parse();
    println!("Value for load_source_cache: {}", opts.load_source_cache);
}

Example output with 3.0.0-beta.2:

❯ cargo run -q
load_source_cache: false
❯ LOAD_SOURCE_CACHE=1 cargo run -q
load_source_cache: true
❯ LOAD_SOURCE_CACHE=0 cargo run -q
load_source_cache: true
❯ LOAD_SOURCE_CACHE=false cargo run -q
load_source_cache: true
❯ LOAD_SOURCE_CACHE=true cargo run -q
load_source_cache: true

Describe the solution you'd like

What I would like to be able to specify in the files instead is:

# base/values.yaml (base file)
envVars:
  SOME_ARGUMENT: "SOME VALUE"
  LOAD_SOURCE_CACHE: "false"

# prod/values.yaml
envVars:
  LOAD_SOURCE_CACHE: "true"

# staging/values.yaml
envVars:
  LOAD_SOURCE_CACHE: "false"

I.e. the value set for the "flag env var" should be specified with:

  • "1" / "0"
  • "true" / "false"
  • "yes" / "no"

Any other value would be ignored (maybe empty value would mean false).

Alternatives, if applicable

No response

Additional Context

No response

@rami3l
Copy link
Contributor

rami3l commented Jul 30, 2021

@epage @pksunkara

Thoughts: it seems that Python developers have a rather handy function strtobool to handle this:

True values are y, yes, t, true, on and 1; false values are n, no, f, false, off and 0. Raises ValueError if val is anything else.

I think this mechanism will suit most of the cases.

The current possible ways to implement a flag (before #2619) are:

  1. --flag (determined by presence) in CLI, FLAG=anything in env
  2. The pseudo-flag pattern, which gives the library user greater freedom to choose what they want for the input

I propose that we can have in the future:

  1. --flag + --flag=bool (bool is determined by strtobool) in CLI, FLAG=bool in env
  2. The pseudo-flag pattern (remains unchanged)

I'm aware of the fact that for CLI flags there is a separate issue #1649, so first I'd like to focus on the possibility of using the strtobool approach just for env args.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants