Fix publication checksum issue #1674
Conversation
|
Warning: Issue #4458 is not at NEW/ASSIGNED/POST. |
There was a problem hiding this comment.
"error": {
"description": "'NoneType' object has no attribute 'get'",
"traceback": " File \"/usr/local/lib/pulp/lib64/python3.6/site-packages/rq/worker.py\", line 822, in perform_job\n rv = job.perform()\n File \"/usr/local/lib/pulp/lib64/python3.6/site-packages/rq/job.py\", line 605, in perform\n self._result = self._execute()\n File \"/usr/local/lib/pulp/lib64/python3.6/site-packages/rq/job.py\", line 611, in _execute\n return self.func(*self.args, **self.kwargs)\n File \"/home/vagrant/devel/pulp-2to3-migration/pulp_2to3_migration/app/tasks/migrate.py\", line 122, in migrate_from_pulp2\n loop.run_until_complete(migrate_distributors(plan))\n File \"/usr/lib64/python3.6/asyncio/base_events.py\", line 484, in run_until_complete\n return future.result()\n File \"/home/vagrant/devel/pulp-2to3-migration/pulp_2to3_migration/app/migration.py\", line 199, in migrate_distributors\n await migrate_repo_distributor(pb, dist_migrator, pulp2dist)\n File \"/home/vagrant/devel/pulp-2to3-migration/pulp_2to3_migration/app/migration.py\", line 159, in migrate_repo_distributor\n pulp2dist, repo_version)\n File \"/home/vagrant/devel/pulp-2to3-migration/pulp_2to3_migration/app/plugin/rpm/repository.py\", line 64, in migrate_to_pulp3\n publish(repo_version.pk)\n File \"/home/vagrant/devel/pulp_rpm/pulp_rpm/app/tasks/publishing.py\", line 222, in publish\n publication.package_checksum_type = checksum_types.get(\"package\")\n"
| @@ -219,8 +219,8 @@ def publish(repository_version_pk, metadata_signing_service=None, checksum_types | |||
|
|
|||
There was a problem hiding this comment.
At the top of the function, you need to add checksum_types = checksum_types or {} or the verbose equivalent
fao89
force-pushed
the
6505
branch
2 times, most recently
from
e6e9df6
to
fd0b2f0
Compare
pulp_rpm/app/tasks/synchronizing.py
Outdated
| @@ -656,6 +656,7 @@ async def run(self): | |||
| for pkg in packages.values(): | |||
| package = Package(**Package.createrepo_to_dict(pkg)) | |||
| artifact = Artifact(size=package.size_package) | |||
| checksums["package_checksum_type"] = package.checksum_type | |||
There was a problem hiding this comment.
Saving original package_checksum_type to use it as default when publishing
There was a problem hiding this comment.
there is notneed to iterate through all the packages here this is similar to #1655 (comment)
in addition, on the Package model we already have checksum_type that is used in case no checksum_type is specified https://github.com/pulp/pulp_rpm/blob/master/pulp_rpm/app/models/package.py#L357
I do not think we need "package_checksum_type" at all
| @@ -211,6 +211,12 @@ def publish(repository_version_pk, metadata_signing_service=None, checksum_types | |||
| """ | |||
| repository_version = RepositoryVersion.objects.get(pk=repository_version_pk) | |||
| repository = repository_version.repository.cast() | |||
| checksum_types = checksum_types or {} | |||
| checksum_types["original"] = repository.original_checksum_types | |||
There was a problem hiding this comment.
i know that maybe i am looking to far away in the future, but this code will not work if the content of the created repo was uploaded. original_checksum_types are set during sync, we would need to add same code to the upload workflow then. Also, during the migration of the repo these values are obviously not set
I was wondering if taking this approach would not be easier and more explicit:
if not checksum_types:
checksum_types["package"] = CHECKSUM_TYPES.SHA256
checksum_types["metadata"] = CHECKSUM_TYPES.SHA256
Also you can get rid of of the complex structure you have in the 'original'
During sync it is enough to inspect only `primary' and take the checksum_type value from it, thend uring publish you can simplify the code here
def get_checksum_types(checksum_types):
"""
Get checksum algorithm for publishing metadata.
Args:
checksum_types (dict): Checksum types for metadata and packages.
"""
original = checksum_types.get("original", CHECKSUM_TYPES.SHA256)
metadata = checksum_types.get("metadata")
if metadata:
checksum_type = metadata
else:
checksum_type = original
return getattr(cr, checksum_type.upper(), cr.SHA256)
There was a problem hiding this comment.
@fabricio-aguiar nevermind, let's just get rid of package_checksum_type so please remove this line:
original_package_checksum_type = repository.original_checksum_types.get(
"package_checksum_type", CHECKSUM_TYPES.SHA256)
But edit this one:
publication.package_checksum_type = checksum_types.get(
"package", CHECKSUM_TYPES.SHA256)
There was a problem hiding this comment.
please address these 2 comments:
https://github.com/pulp/pulp_rpm/pull/1674/files#r410125559
https://github.com/pulp/pulp_rpm/pull/1674/files#r410165630
https://pulp.plan.io/issues/6505