Added label command to ansible distribution #186
Conversation
pulpcore/cli/ansible/distribution.py
Outdated
| @@ -79,6 +80,7 @@ def distribution(ctx: click.Context, pulp_ctx: PulpContext, distribution_type: s | |||
| distribution.add_command(show_command(decorators=lookup_options)) | |||
| distribution.add_command(destroy_command(decorators=lookup_options)) | |||
| distribution.add_command(create_command(decorators=create_options)) | |||
| distribution.add_command(label_command()) | |||
There was a problem hiding this comment.
This is added in an upcoming pulp_ansible version, right?
Can we pass a list of needed plugins with versions to the label_command generator?
And than we should have a test that can excercise this at least on the nightly container.
gerrod3
force-pushed
the
ansible_distribution_labels
branch
2 times, most recently
from
2818ea8 to
2c62fe6
Compare
pulpcore/cli/common/generic.py
Outdated
| @@ -390,10 +390,15 @@ def label_command(**kwargs: Any) -> click.Command: | |||
| if "name" not in kwargs: | |||
| kwargs["name"] = "label" | |||
| decorators = kwargs.pop("decorators", [name_option, href_option]) | |||
| needs = kwargs.pop("needs", [("core", "3.10.0")]) | |||
There was a problem hiding this comment.
I'd prefer a more descriptive name: need_plugins, needed_plugins, ...
There was a problem hiding this comment.
For both the kwargs and the variable name?
There was a problem hiding this comment.
Yes. Sure they should be the same.
Which one do you prefer?
pulpcore/cli/common/generic.py
Outdated
| min_ver = plugin[1] if len(plugin) >= 2 else None | ||
| max_ver = plugin[2] if len(plugin) == 3 else None | ||
| pulp_ctx.needs_plugin(plugin[0], min_ver, max_ver) |
There was a problem hiding this comment.
We can do something fancy with a namedtuple here. But maybe not today...
There was a problem hiding this comment.
Ask and you shall receive.
gerrod3
force-pushed
the
ansible_distribution_labels
branch
2 times, most recently
from
65e604b to
6885ef8
Compare
pulpcore/cli/common/generic.py
Outdated
| for item in need_plugins: | ||
| pulp_ctx.needs_plugin(item.name, item.min, item.max) |
There was a problem hiding this comment.
Would this work?
| for item in need_plugins: | |
| pulp_ctx.needs_plugin(item.name, item.min, item.max) | |
| for item in need_plugins: | |
| pulp_ctx.needs_plugin(*item) |
or this?
| for item in need_plugins: | |
| pulp_ctx.needs_plugin(item.name, item.min, item.max) | |
| [pulp_ctx.needs_plugin(*item) for item in need_plugins] |
There was a problem hiding this comment.
- Yes if I edit the tuple. 2. Yes if I edit the tuple, but mypy does not like it since
needs_plugindoesn't return anything.
(edit) NVM, don't need to edit the tuple since the args are in the same order I declared the name fields.
There was a problem hiding this comment.
yep, i agree the latter is rather unintuive. Usually you use list comprehensions that do not have side effects. This would be only using the side effect.
gerrod3
force-pushed
the
ansible_distribution_labels
branch
from
6885ef8 to
bdaccfa
Compare
I merged the ansible labels feature Wednesday, so it should I think. |
fixes: #185