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

CLI arguments should use a standarized data structure and interface #8

Closed
mtreinish opened this issue Dec 31, 2016 · 3 comments
Closed

Comments

@mtreinish
Copy link
Owner

Currently the stestr.cli module passes the raw output from argparse's parse_known_args() method to the run() method in each command module. While this works it's not a very friendly interface and it makes using the commands via a python interface a bit more complicated since you'd have to construct a tuple with the argparse Namespace object and any additional parameters to pass arguments into the run() methods.

To really make the commands externally consumable via a python api we need to standardize how we pass arguments into these functions. Whether that consists of switching from the tuple with the Namespace and list to something like a dict, or leaving it as is doesn't really matter too much. We just need a clearly writtend doc defining how parameters are passed in and having examples on how to use it outside of stestr.cli.

@mtreinish
Copy link
Owner Author

The idea I had in mind right now is for the commands run() function have that call a function that generates a couple of dicts with a known schema (maybe using voluptuous to define it, but that's probably overkill) that can be used by an inner function that can be called with at least one of those dicts used as **kwargs directly. That way for python consumers they can just call the inner functions from outside of stestr that will have a defined set of kwargs they can use. Then for the interface docs we document how to call those inner functions.

Where this gets a bit complicated is when you look at the call stack and how variables are accessing the arguments object. Right now it's kinda used a dumping ground for local state. (it was much easier to just put everything in there and pass the one object around) So things which aren't checked inside the commands/*.py module get used 2 or 3 calls down the stack. So we have to make sure that all those bits are clearly exposed at the top level and at each layer going down the stack.

@mtreinish
Copy link
Owner Author

I've started working on this idea in my wip branch: https://github.com/mtreinish/stestr/tree/add_python_interface it's still far from complete, but it's the starting point. (also it's untested at this point as well) I'll continue to update that branch as I work on it. I'll PR it when I think it's ready. If someone would like to help on that feel free to just ask, I'm not sure how fast I'll be able to move on it.

mtreinish added a commit that referenced this issue Jul 24, 2017
This commit adds a real api for running the stestr commands. Instead of
just having a magic run() function that takes in an tuple of a argparse
Namespace and a list of undefined arguments this migrates all the real
work into a function that has properly defined kwargs. The run() command
is then relegated to just convert the Namespace into a dict and pass the
arguments into that real function. This enables external programs to
just call the new functions with defined args and run commands exactly
like on the cli, but with a defined python interface. It makes
everything a lot easier for python consumption. The tradeoff here though
is that everything is bit more verbose, but that's the cost of being
explicit with a defined interface.

As a side effect of this change instead of passing that Namespace object
around between all the lower layers real interfaces have to be defined
for all the functions. This means a ton of new kwargs, but again this
is better in the long run because it means we have defined interfaces
for all the functions.

Closes Issue #8
mtreinish added a commit that referenced this issue Jul 24, 2017
This commit adds a real api for running the stestr commands. Instead of
just having a magic run() function that takes in an tuple of a argparse
Namespace and a list of undefined arguments this migrates all the real
work into a function that has properly defined kwargs. The run() command
is then relegated to just convert the Namespace into a dict and pass the
arguments into that real function. This enables external programs to
just call the new functions with defined args and run commands exactly
like on the cli, but with a defined python interface. It makes
everything a lot easier for python consumption. The tradeoff here though
is that everything is bit more verbose, but that's the cost of being
explicit with a defined interface.

As a side effect of this change instead of passing that Namespace object
around between all the lower layers real interfaces have to be defined
for all the functions. This means a ton of new kwargs, but again this
is better in the long run because it means we have defined interfaces
for all the functions.

Closes Issue #8
mtreinish added a commit that referenced this issue Jul 25, 2017
This commit adds a real api for running the stestr commands. Instead of
just having a magic run() function that takes in an tuple of a argparse
Namespace and a list of undefined arguments this migrates all the real
work into a function that has properly defined kwargs. The run() command
is then relegated to just convert the Namespace into a dict and pass the
arguments into that real function. This enables external programs to
just call the new functions with defined args and run commands exactly
like on the cli, but with a defined python interface. It makes
everything a lot easier for python consumption. The tradeoff here though
is that everything is bit more verbose, but that's the cost of being
explicit with a defined interface.

As a side effect of this change instead of passing that Namespace object
around between all the lower layers real interfaces have to be defined
for all the functions. This means a ton of new kwargs, but again this
is better in the long run because it means we have defined interfaces
for all the functions.

Closes Issue #8
mtreinish added a commit that referenced this issue Jul 25, 2017
This commit adds a real api for running the stestr commands. Instead of
just having a magic run() function that takes in an tuple of a argparse
Namespace and a list of undefined arguments this migrates all the real
work into a function that has properly defined kwargs. The run() command
is then relegated to just convert the Namespace into a dict and pass the
arguments into that real function. This enables external programs to
just call the new functions with defined args and run commands exactly
like on the cli, but with a defined python interface. It makes
everything a lot easier for python consumption. The tradeoff here though
is that everything is bit more verbose, but that's the cost of being
explicit with a defined interface.

As a side effect of this change instead of passing that Namespace object
around between all the lower layers real interfaces have to be defined
for all the functions. This means a ton of new kwargs, but again this
is better in the long run because it means we have defined interfaces
for all the functions.

Closes Issue #8
mtreinish added a commit that referenced this issue Jul 26, 2017
This commit adds a real api for running the stestr commands. Instead of
just having a magic run() function that takes in an tuple of a argparse
Namespace and a list of undefined arguments this migrates all the real
work into a function that has properly defined kwargs. The run() command
is then relegated to just convert the Namespace into a dict and pass the
arguments into that real function. This enables external programs to
just call the new functions with defined args and run commands exactly
like on the cli, but with a defined python interface. It makes
everything a lot easier for python consumption. The tradeoff here though
is that everything is bit more verbose, but that's the cost of being
explicit with a defined interface.

As a side effect of this change instead of passing that Namespace object
around between all the lower layers real interfaces have to be defined
for all the functions. This means a ton of new kwargs, but again this
is better in the long run because it means we have defined interfaces
for all the functions.

Closes Issue #8
@mtreinish
Copy link
Owner Author

With #66 merged I think this bug is now. I still need to add documentation on how to use the interfaces but I will do that in later patches

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant