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.

Sign up for GitHub

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

Jump to bottom

Add pulp file ACS refresh command #377

Merged
merged 1 commit into from
Sep 27, 2021
Merged

Add pulp file ACS refresh command #377

merged 1 commit into from
Sep 27, 2021

Conversation

daviddavis
Copy link
Contributor

[noissue]

@daviddavis daviddavis force-pushed the acs-refresh branch 7 times, most recently from a939725 to ae431f9 Compare September 22, 2021 19:22
@daviddavis daviddavis linked an issue Sep 22, 2021 that may be closed by this pull request
Handle TaskGroup response #366
Closed
mdellweg
mdellweg reviewed Sep 23, 2021
View reviewed changes
Copy link
Member

@mdellweg mdellweg left a comment

Choose a reason for hiding this comment

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

I'd like to have one if the ACS experts to judge the necessary test coverage here.

pulpcore/cli/common/context.py Show resolved Hide resolved
pulpcore/cli/common/context.py Outdated Show resolved Hide resolved
pulpcore/cli/core/task.py Show resolved Hide resolved
pulpcore/cli/file/context.py Outdated Show resolved Hide resolved
@daviddavis daviddavis force-pushed the acs-refresh branch 4 times, most recently from c01642a to 2e3d083 Compare September 23, 2021 18:24
@daviddavis
Copy link
Contributor Author

@pavelpicka can you review?

pavelpicka
pavelpicka approved these changes Sep 24, 2021
View reviewed changes
Copy link
Collaborator

@pavelpicka pavelpicka left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM
just "cli-bad-repo" in tests invokes malformed repo, this case sounds like incomplete or metadata only (if we expect success)
and one negative test would be nice to see cli can handle it.

@daviddavis
Copy link
Contributor Author

I renamed the repo cli-repo-manifest-only and added a failing refresh test.

@mdellweg
Copy link
Member

Are the necessary bits for this merged to pulpcore and pulp_file or do we need to wait with merging to not break their respective ci?

@daviddavis
Copy link
Contributor Author

Are the necessary bits for this merged to pulpcore and pulp_file or do we need to wait with merging to not break their respective ci?

The only thing this relies on is the pulp_file refresh command which got merged: pulp/pulp_file#536

@mdellweg mdellweg merged commit a928084 into pulp:develop Sep 27, 2021
@mdellweg mdellweg added this to the 0.12.0 milestone Sep 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pavelpicka pavelpicka pavelpicka approved these changes

@mdellweg mdellweg mdellweg approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.12.0
Development

Successfully merging this pull request may close these issues.

Handle TaskGroup response
3 participants
@daviddavis @mdellweg @pavelpicka