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

Validate arguments in conf file when passed to CLI #1793

Closed
merelcht opened this issue Aug 17, 2022 · 4 comments · Fixed by #3285
Closed

Validate arguments in conf file when passed to CLI #1793

merelcht opened this issue Aug 17, 2022 · 4 comments · Fixed by #3285
Assignees
Labels
Component: CLI Issue/PR that addresses the CLI for Kedro

Comments

@merelcht
Copy link
Member

merelcht commented Aug 17, 2022

Description

When using kedro run --config no errors are thrown if the arguments in the provided run config file are wrong, e.g.

run:
  from-nodes: bla # this should be `from_nodes`

doesn't result in an error, but if you run kedro run --from_nodes bla (this should be kedro run --from-nodes) it will say:

Error: No such option: --from_nodes (Possible options: --from-nodes, --nodes, --to-nodes)

Context

#1485

Task

Validate the provided arguments are the expected arguments from the run function, optionally print out what's the argument it has parsed. Currently, if we provide an invalid argument it will still run but doesn't do anything, it is hard to debug, especially the CLI argument can override the YAML config. Some investigation needs to be done to check how feasible this is and what the current behaviour is (e.g. will it throw an error if you misspell a node in --from-nodes my_node1 or is it ignored?) to make sure it's consistent.

@noklam
Copy link
Contributor

noklam commented Aug 17, 2022

Adding some more context here:
I think this is specifically related to the --config argument, where it doesn't fail if you provide an arbitrary key. That's part of the reason why from_node is confusing because if people passing from-node it would still run but without any effect.

  • This could be mitigated with the kedro compile proposal, or a similar discussion here to output a compiled version of kedro parameters/catalog, I think this is becoming more relevant with experiment-tracking, where you want to have a diff of your parameters.
  • It's still nice to have validation from the kedro side, we may validate a hard-coded list of keywords (may not work well if the user has a custom run command, maybe a rare case though I actually did this before), or we can actually access the argument through click's context.

@deepyaman
Copy link
Member

Adding some more context here: I think this is specifically related to the --config argument, where it doesn't fail if you provide an arbitrary key. That's part of the reason why from_node is confusing because if people passing from-node it would still run but without any effect.

On the specific note w.r.t. hyphens/underscores, I just came across https://jwodder.github.io/kbits/posts/click-config/, which says:

Note that, with this code, parameters in the config file must be named using the same name & spelling as the parameter’s corresponding argument to the command callback. For example, the --integer option must be written integer, not --integer or -i or i; any entries in the config file with an invalid spelling will be ignored. For options with medial hyphens on the command line, like --log-level, the hyphens must become underscores in the configuration file, like log_level. If you want to support the spelling log-level as well, insert the following line after cfg = ConfigParser() to make the ConfigParser object convert hyphens in option names to underscores:

cfg.optionxform = lambda s: s.replace('-', '_')

@merelcht merelcht added this to the Configuration overhaul milestone Jan 10, 2023
@merelcht merelcht changed the title Validate conf arguments passed to CLI Validate arguments in conf file when passed to CLI Sep 5, 2023
@merelcht merelcht moved this to To Do in Kedro Framework Nov 1, 2023
@merelcht merelcht self-assigned this Nov 7, 2023
@merelcht merelcht moved this from To Do to In Progress in Kedro Framework Nov 7, 2023
@merelcht merelcht linked a pull request Nov 7, 2023 that will close this issue
7 tasks
@noklam
Copy link
Contributor

noklam commented Nov 8, 2023

image
When there is error it suggests using --verbose which doesn't exist.

I actually wish there is a --verbose command that automatically switch to DEBUG level log.

@merelcht merelcht moved this from In Progress to In Review in Kedro Framework Nov 9, 2023
@merelcht
Copy link
Member Author

Completed in #3285

@github-project-automation github-project-automation bot moved this from In Review to Done in Kedro Framework Nov 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: CLI Issue/PR that addresses the CLI for Kedro
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants