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

Checksum type check when publish #1890

Merged

dralley merged 1 commit into pulp:master from pavelpicka:7855-checksum-type-artifact

Feb 4, 2021

Contributor

pavelpicka commented Nov 24, 2020 •

edited

Loading

Check allowed checksum types when publish.

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

Required PR: pulp/pulp-smash#1248

[nocoverage]

pavelpicka changed the title ~~Checksum type and choices constants~~ [WIP] Checksum type and choices constants

pavelpicka force-pushed the 7855-checksum-type-artifact branch from 6430ae3 to 58cff94 Compare

November 24, 2020 08:43

Member

pulpbot commented Nov 24, 2020

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

pavelpicka force-pushed the 7855-checksum-type-artifact branch 7 times, most recently from d702b4a to d5d0ee0 Compare

November 24, 2020 14:38

pavelpicka changed the title ~~[WIP] Checksum type and choices constants~~ Checksum type and choices constants

Contributor

daviddavis commented Nov 24, 2020 •

edited

Loading

Looks good so far. A couple thoughts though. If you alter ALLOWED_CONTENT_CHECKSUMS and run "pulpcore-manager makemigrations" do you get a new migration? The problem is that every time you alter the setting, django wants to alter the database. See https://pulp.plan.io/issues/7776.

I would recommend just setting the model field choices to the checksums in ALL_KNOWN_CONTENT_CHECKSUMS and then validating the checksums in the serializer here against ALLOWED_CONTENT_CHECKSUMS. For Package, you could maybe add a pre-save hook if you wanted.

I would also try to add a test. You should be able to set ALLOWED_CONTENT_CHECKSUMS in .travis/settings.py.j2. Perhaps disinclude md5 and then try to use it to create a publication.

pavelpicka force-pushed the 7855-checksum-type-artifact branch 9 times, most recently from 20a188f to 1b15990 Compare

November 25, 2020 18:35

daviddavis reviewed

View reviewed changes

.ci/ansible/settings.py.j2 Show resolved Hide resolved

daviddavis reviewed

View reviewed changes

pulp_rpm/app/models/package.py Outdated Show resolved Hide resolved

daviddavis reviewed

View reviewed changes

pulp_rpm/app/models/package.py Outdated Show resolved Hide resolved

daviddavis reviewed

View reviewed changes

pulp_rpm/app/models/package.py Outdated Show resolved Hide resolved

daviddavis reviewed

View reviewed changes

pulp_rpm/app/serializers/repository.py Outdated Show resolved Hide resolved

daviddavis reviewed

View reviewed changes

pulp_rpm/tests/functional/api/test_publish.py Show resolved Hide resolved

daviddavis reviewed

View reviewed changes

pulp_rpm/tests/functional/api/test_publish.py Show resolved Hide resolved

pavelpicka force-pushed the 7855-checksum-type-artifact branch 2 times, most recently from f24f6bd to 7324ba4 Compare

November 26, 2020 14:41

pavelpicka changed the title ~~[wip] Checksum type check when publish~~ Checksum type check when publish

pavelpicka force-pushed the 7855-checksum-type-artifact branch 2 times, most recently from e7e97d2 to 67a4477 Compare

December 1, 2020 17:41

goosemania reviewed

View reviewed changes

pulp_rpm/app/serializers/repository.py Show resolved Hide resolved

pavelpicka force-pushed the 7855-checksum-type-artifact branch 5 times, most recently from 031c5ea to f94f7ce Compare

December 14, 2020 16:47

goosemania reviewed

View reviewed changes

pulp_rpm/tests/functional/api/test_publish.py Outdated Show resolved Hide resolved

goosemania reviewed

View reviewed changes

pulp_rpm/app/tasks/publishing.py Outdated Show resolved Hide resolved

pulp_rpm/app/tasks/publishing.py Outdated Show resolved Hide resolved

pulp_rpm/app/serializers/repository.py Outdated Show resolved Hide resolved

pavelpicka force-pushed the 7855-checksum-type-artifact branch 2 times, most recently from 37c6e96 to 1b91770 Compare

January 13, 2021 16:36

goosemania previously approved these changes

View reviewed changes

Member

goosemania left a comment

Thanks! Just some typos, all good.

pulp_rpm/app/tasks/publishing.py Outdated Show resolved Hide resolved

pulp_rpm/tests/functional/api/test_publish.py Outdated Show resolved Hide resolved

pulp_rpm/app/serializers/repository.py Outdated Show resolved Hide resolved

dralley reviewed

View reviewed changes

pulp_rpm/app/tasks/publishing.py Outdated Show resolved Hide resolved

pavelpicka force-pushed the 7855-checksum-type-artifact branch 3 times, most recently from 6d9bc14 to e54ad03 Compare

January 28, 2021 13:04

goosemania reviewed

View reviewed changes

pulp_rpm/app/serializers/repository.py Outdated Show resolved Hide resolved

goosemania self-requested a review

February 2, 2021 10:13

goosemania dismissed their stale review

February 2, 2021 10:14

changes are not in place

pavelpicka force-pushed the 7855-checksum-type-artifact branch 4 times, most recently from f743063 to b08ac52 Compare

February 3, 2021 17:42

goosemania approved these changes

View reviewed changes

Member

goosemania left a comment

Thanks!

pavelpicka force-pushed the 7855-checksum-type-artifact branch from b08ac52 to cb1b421 Compare

February 3, 2021 20:25


          Checksum type check when publish

c306130

Check allowed checksum types when publish.

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

[nocoverage]

pavelpicka force-pushed the 7855-checksum-type-artifact branch from cb1b421 to c306130 Compare

February 4, 2021 15:02

dralley merged commit ad19ad2 into pulp:master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment