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

Fixed package serialization so it displays content checksums. #1911

Merged
merged 1 commit into from
Dec 18, 2020

Conversation

ipanova
Copy link
Member

@ipanova ipanova commented Dec 18, 2020

closes #8002
https://pulp.plan.io/issues/8002

[notest]
[nocoverage]

@pulpbot
Copy link
Member

pulpbot commented Dec 18, 2020

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

@dralley
Copy link
Contributor

dralley commented Dec 18, 2020

There's probably an argument for making SingleArtifactContentSerializer [0] just inherit this as a mixin so it would be automatically included without needing to add it for each content type and each plugin. I'd be OK with it either way.

[0] https://github.com/pulp/pulpcore/blob/master/pulpcore/app/serializers/content.py#L26

As mentioned in IRC, I kinda wonder if we should be putting these checksums under a different namespace to avoid conflicts with fields on the content models, e.g. pulp_ prefix. That is probably a separate discussion and a separate issue. Doing so would eliminate the downside of including it automatically, though.

@ipanova
Copy link
Member Author

ipanova commented Dec 18, 2020

@dralley this is indeed something worth discussing. Not every plugin would benefit from having checksums automatically included in the SingleArtifactContentSerializer. For example pulp_container plugin mostly cares about sha256 only and in case ContenChecksums serializer wiould be incorporated in to the SingleArtifactContentSerializer we would need to exclude all checksums form serialization except sha256.

@ipanova ipanova merged commit 85eb931 into pulp:master Dec 18, 2020
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

3 participants