Open jtarchie opened 6 years ago
We have created an issue in Pivotal Tracker to manage this. Unfortunately, the Pivotal Tracker project is private so you may be unable to view the contents of the story.
The labels on this github issue will be updated when the story is started.
This seems interesting as a more holistic solution to flag parsing. The only issue that jumps out immediately is --config
is overloaded as commands like configure-product
already have a --config
flag. Maybe there's a better name? Changing the name of the existing --config
flags as a breaking change is also an option if we feel like --config
is the best name for the flag that reads flags.
@ljfranklin, the implication above is that it would be a backwards compatible change with --config
. Just moving the logic that those commands have for loading directly from a single file from om
to jhanda
.
Of course, when I say this, I do realize this would affect the interpolation that om
does on those files too. 🤔
I don't have a good feel for whether this belongs in here, or if it'll take some time to gain alignment on the design.
But if it's feeling like it's going to take time to hash out the design or decide, one option in the short term may be to first wrap jhanda inom
to add this behavior for now. That could solve the om
problem of centralizing this responsibility and then offer a reference implementation to consider pushing down into jhanda
.
the implication above is that it would be a backwards compatible change with --config
Right, I forgot that the config file that configure-product
takes in has top-level keys that match the flag names. We do have other commands like create-vm-extension
where this doesn't hold:
ॐ create-vm-extension
This creates/updates a VM extension
Usage: om [options] configure-product [<args>]
--client-id, -c, OM_CLIENT_ID string Client ID for the Ops Manager VM (not required for unauthenticated commands)
--client-secret, -s, OM_CLIENT_SECRET string Client Secret for the Ops Manager VM (not required for unauthenticated commands)
--connect-timeout, -o int timeout in seconds to make TCP connections (default: 5)
--format, -f string Format to print as (options: table,json) (default: table)
--help, -h bool prints this usage information (default: false)
--password, -p, OM_PASSWORD string admin password for the Ops Manager VM (not required for unauthenticated commands)
--request-timeout, -r int timeout in seconds for HTTP requests to Ops Manager (default: 1800)
--skip-ssl-validation, -k bool skip ssl certificate validation during http requests (default: false)
--target, -t, OM_TARGET string location of the Ops Manager VM
--trace, -tr bool prints HTTP requests and response payloads
--username, -u, OM_USERNAME string admin username for the Ops Manager VM (not required for unauthenticated commands)
--version, -v bool prints the om release version (default: false)
Command Arguments:
--cloud-properties, -cp string (required) cloud properties in JSON format
--config, -c string path to yml file containing all config fields (see docs/create-vm-extension/README.md for format)
--name, -n string (required) VM extension name
--ops-file, -o string (variadic) YAML operations file
--vars-file, -l string (variadic) Load variables from a YAML file
vm-extension-config:
name: some_vm_extension
cloud_properties:
source_dest_check: false
But changing the top-level key in the create-vm-extension
config file to match the flag name or vice versa seems doable.
The command line arguments being used in
om
sometimes represent large JSON payloads. For example, theconfigure-product --product-properties
. This command has added support for a--config
YAML file, so those command line arguments can be passed in -- over bash encoding a large JSON string.We were investigating adding this behaviour to other commands on
om
, too. Since,--config
file is mapping a file to command line arguments, we thinkjhanda
might be a great place to centralize this logic.Our team's proposal, is to have a special
ConfigFile
type for that if it exists on thestruct
will automatically load from the file.Better to show than tell. If we were to write a test.