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

Feature Request: Automatically Pass Globals to Variant Modules #29

Closed
osterman opened this issue Sep 18, 2020 · 2 comments · Fixed by #30
Closed

Feature Request: Automatically Pass Globals to Variant Modules #29

osterman opened this issue Sep 18, 2020 · 2 comments · Fixed by #30

Comments

@osterman
Copy link

what

  • we rely on a lot of module imports and globals
  • All imported modules have parameters with defaults
  • Using default values global parameters in the root variant module has no effect on the settings of the parameters in the imported modules. However, using explicit command-line arguments does work, but results in massive commands

example

Our root module has something like this:

imports = [
  "cli/modules",
  "cli/modules/shell",
  "cli/modules/kubeconfig",
  "cli/modules/terraform",
  "cli/modules/helmfile",
  "cli/modules/helm",
  "cli/modules/workflow",
]

Then in the root module we define a bunch of global options

option "region" {
  default = "us-east-1"
  description = "AWS region"
  type = string
}

option "dry-run" {
  default = false
  description = "Disable execution of any commands and echo the commands instead"
  type = bool
}

option "kubeconfig-path" {
  default = "/dev/shm"
  description = "folder to save kubeconfig"
  type = string
}

option "cluster-name-pattern" {
  default = "ie-$$environment-$$stage-eks-cluster"
  description = "Cluster name pattern"
  type = string
}

option "config-dir" {
  # The default works from geodesic shell
  default = "/foobar/config"
  description = "Config directory"
  type = string
}

Then in one of our imported modules will have something like: (note the defaults are different)

option "region" {
  default = "us-west-2"
  description = "AWS region"
  type = string
}

option "dry-run" {
  default = false
  description = "Disable execution of any commands and echo the commands instead"
  type = bool
}

option "kubeconfig-path" {
  default = "/dev/shm"
  description = "folder to save kubeconfig"
  type = string
}

option "cluster-name-pattern" {
  default = "eg-$$environment-$$stage-eks-cluster"
  description = "Cluster name pattern"
  type = string
}

option "config-dir" {
  # The default works from geodesic shell
  default = "/cli/config"
  description = "Config directory"
  type = string
}

We want the root global defaults to override the module defaults. Meaning that if --config-dir is not passed on the command line, then the default value of the option in the root module, should override the default value of the imported module. If the imported module does not declare the option/parameter, then it's just skipped. So only modules that explicitly define the parameter will inherit the default value from the root module.

why

  • We want to be able to override the defaults for parameters at the global level so that it can be customized per cli. For example, the defaults for one customer are different for another customer.
  • This used to work before supporting multiple imports (with an s)
@osterman
Copy link
Author

@mumoshu wrote (in slack):

i remember that i though each module should have a way to opt-in which global param/opt to get propagated, hence that change. of course breaking existing usecase and making variant files too verbose isn't my purpose we should definitely revisit it

I am okay if there's a way to be more explicit about when the overwriting/inheritance takes place

Maybe something like this:

option "region" {
  default = "us-west-2"
  description = "AWS region"
  type = string
  global = true
}

Unless global is true, it does not overwrite default of module?

@mumoshu wrote:

are you okay on permitting a module having access to global options and parameters that are not literally defined within the module?

I think modules must define options/parameters explicitly.

@mumoshu wrote:

opt/param values should be propagated as long as the module has same opt/parm definitions. and the module should be able to skip defaults

Exactly.

mumoshu added a commit that referenced this issue Sep 23, 2020
Imported variant command's globals (top-level parameters and options) are now merged into the command, when it misses parameters and options with the same names.

If any imported global's type conflict with the command's corresponding parameter/option type, it fails early so that the imported command doesn't fail due to unexpected value/type provided.

Resolves #29
mumoshu added a commit that referenced this issue Sep 23, 2020
Imported variant command's globals (top-level parameters and options) are now merged into the command, when it misses parameters and options with the same names.

If any imported global's type conflict with the command's corresponding parameter/option type, it fails early so that the imported command doesn't fail due to unexpected value/type provided.

Resolves #29
mumoshu added a commit that referenced this issue Sep 24, 2020
Imported variant command's globals (top-level parameters and options) are now merged into the command, when it misses parameters and options with the same names.

If any imported global's type conflict with the command's corresponding parameter/option type, it fails early so that the imported command doesn't fail due to unexpected value/type provided.

Resolves #29
@mumoshu
Copy link
Owner

mumoshu commented Sep 26, 2020

@osterman Thank you so much for the detailed request! I thought I've made it work like that when I implemented the first version of import functionality, but apparently not.

#30 has been merged to fix it.

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 a pull request may close this issue.

2 participants