mumoshu / variant2

Turn your bash scripts into a modern, single-executable CLI app today
MIT License
141 stars 11 forks source link

Feature Request: Automatically Pass Globals to Variant Modules #29

Closed osterman closed 3 years ago

osterman commented 3 years ago

what

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

osterman commented 3 years ago

@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 commented 3 years ago

@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.