-
Notifications
You must be signed in to change notification settings - Fork 42
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
Add the --wait option to the task-group show command #565
Conversation
11842ea
to
6049764
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice. Just a few cosmetic changes.
"task_group_read", parameters={"task_group_href": task_group["pulp_href"]} | ||
"task_groups_read", parameters={"task_group_href": task_group["pulp_href"]} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a bugfix. Didn't we have tests for it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it is a bug fix. I think we do not have any tests for it. The method wait_for_task_group
was not invoked anywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch!
CHANGES/459.feature
Outdated
@@ -0,0 +1,2 @@ | |||
Introduced the option ``--wait`` for the task-group show command. By using this option, details | |||
of a specific task-group will be shown only in case all related tasks will be finished. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
of a specific task-group will be shown only in case all related tasks will be finished. | |
of the task-group will be shown only after waiting for all related tasks to finish. |
Please don't do the crazy wrapping here. Wrap whole sentences instead.
tests/scripts/pulp_file/test_acs.sh
Outdated
@@ -37,7 +37,12 @@ test "$(echo "$OUTPUT" | jq ".paths | length")" -eq 2 | |||
# test refresh | |||
expect_succ pulp file acs refresh --name $acs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe add --background
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will "ERROUTPUT" be still accessible on the next line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will only be overwritten on the next "expect_*"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or you mean that "--background" still tells you about the triggered taskgroup? I hope so.
pulpcore/cli/core/task_group.py
Outdated
@@ -18,3 +28,49 @@ def task_group(ctx: click.Context, pulp_ctx: PulpContext) -> None: | |||
|
|||
task_group.add_command(list_command()) | |||
task_group.add_command(show_command(decorators=[href_option])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This ought to go now.
6049764
to
c3d81fb
Compare
c3d81fb
to
6070b47
Compare
closes #459