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

Universal content list properly excludes PublishedMetadata #1218

Merged
merged 1 commit into from
Mar 31, 2021

Conversation

gerrod3
Copy link
Contributor

@gerrod3 gerrod3 commented Mar 30, 2021

fixes: #8447
https://pulp.plan.io/issues/8447

Not sure how to add tests for this since I don't think pulp_file publications will trigger it.

Please be sure you have read our documentation on creating PRs:
https://docs.pulpproject.org/contributing/pull-request-walkthrough.html

@pulpbot
Copy link
Member

pulpbot commented Mar 30, 2021

Attached issue: https://pulp.plan.io/issues/8447

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.

Code wise 👍

@@ -0,0 +1 @@
Publications that create PublishedMetadata no longer break `/pulp/api/v3/content/`
Copy link
Member

Choose a reason for hiding this comment

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

We agreed, that this changelog should describe what changed in past tense. However i have no real suggestion of the top of my head.

Suggested change
Publications that create PublishedMetadata no longer break `/pulp/api/v3/content/`
Fixed a bug where the existence of PublishedMetadata broke `/pulp/api/v3/content/` by excluding them.

I guess, even if it were able to show them it'd be considered a bug, because PublishedMetadata is not user facing.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would probably just keep it simple:

Suggested change
Publications that create PublishedMetadata no longer break `/pulp/api/v3/content/`
Fixed ``LookupError`` when hitting ``/pulp/api/v3/content/`` with published content.

Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer

Fixed a bug where the existence of PublishedMetadata caused ``LookupErrorwhen querying/pulp/api/v3/content/`

@dralley dralley merged commit 3d3a784 into pulp:master Mar 31, 2021
@gerrod3 gerrod3 deleted the 8447 branch April 2, 2021 02:11
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.

None yet

5 participants