Skip to content

Conversation

@himdel
Copy link
Collaborator

@himdel himdel commented May 22, 2022

This is trying to do a similar thing to #30 , but reducing the scope to just moving all custom param parsing to argparse subparsers with no syntax changes and no DRYing up the actual commands yet.

The effect should be better help, more reliable failures with unexpected param counts, and ability to add named arguments easily.

(help description can be added to each kind, op, subop and arg, but it can be done gradually, mostly leaving out of this PR; It looks #30 has a lot more help text ready.)

(args.function is also out of scope, for now just having the subparser dest= to the original arg name (kind, operation, subop))
(args.rest is gone now)

Renaming args.username & args.password to args.auth_username and args.auth_password, to prevent clashes with the username and password positional arguments for user create. (version would also conflict but main uses action="version", not action="store")

Also adding --debug and --version params, and admin default values for username and password.

@himdel himdel marked this pull request as ready for review May 23, 2022 11:10
@himdel
Copy link
Collaborator Author

himdel commented May 23, 2022

@himdel
Copy link
Collaborator Author

himdel commented May 23, 2022

Cc @hendersonreed this is my attempt to revive the command.py refactor by doing the minimum needed to get argparse parsing all the options instead of us splitting args.rest manually. WDYT? :)

(If this makes sense, we can follow up with taking the command cleanup parts from #30, adding more help strings, etc.)

@hendersonreed hendersonreed self-requested a review May 24, 2022 20:14
Copy link
Contributor

@hendersonreed hendersonreed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is necessary, and all current QE environments using galaxykit are using an older version. If this does break them, we should update them when it comes time (but I don't think it does.)

IMO this is safe to merge to master for sure - all QE use is with versions that are released in PyPI.

@himdel himdel merged commit 7798723 into ansible:main May 26, 2022
@himdel himdel deleted the command-parse branch May 26, 2022 00:08
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 this pull request may close these issues.

2 participants