Refactor sync pipeline #608
Conversation
mdellweg
force-pushed
the
refactor_sync
branch
8 times, most recently
from
7fc3be1
to
ee06a72
Compare
| manifest_list=manifest_dc.content, | ||
| image_manifest=dc.content, |
There was a problem hiding this comment.
Wow, i really think we are using these variables the wrong way around.
There was a problem hiding this comment.
How much effort will it take to rename those variables?
There was a problem hiding this comment.
It involves a data migration at least. I don't want to mix that up in another big PR like this.
mdellweg
force-pushed
the
refactor_sync
branch
2 times, most recently
from
6017733
to
d801f93
Compare
| for manifest_dc in self.manifest_dcs: | ||
| config_blob_dc = manifest_dc.extra_data.get("config_blob_dc") | ||
| if config_blob_dc: | ||
| manifest_dc.content.config_blob = await config_blob_dc.resolution() | ||
| for blob_dc in manifest_dc.extra_data["blob_dcs"]: | ||
| # Just await here. They will be associated in the post_save hook. | ||
| await blob_dc.resolution() | ||
| await self.put(manifest_dc) | ||
| self.manifest_dcs.clear() |
There was a problem hiding this comment.
Can we end up in a state where self.manifest_dcs.clear() will be invoked less frequently than the actual iteration (because of await blob_dc.resolution())? In the end, we will be iterating over the same manifest declarative contents.
There was a problem hiding this comment.
I do not understand the question. We collect the manifest_dc in a bucket, we make sure they are ready to processed, we shove them in the pipeline we empty the bucket and start over.
There is no break, so the only thing that can end the loop prematurely is a fatal exception.
There was a problem hiding this comment.
I meant that if there is any possibility to run resolve_flush for the same declarative content objects. So, we will be calling resolve_flush on declarative content that was already queued.
There was a problem hiding this comment.
It can happen that multiple declarative content objects result in the same content, but every dc instance will only be part of a resolve_flush call once. (That is why we clear the list.)
| manifest_list=manifest_dc.content, | ||
| image_manifest=dc.content, |
There was a problem hiding this comment.
How much effort will it take to rename those variables?
| [ | ||
| self.create_listed_manifest(list_dc, manifest_data) | ||
| for manifest_data in content_data.get("manifests") | ||
| ] |
There was a problem hiding this comment.
Should we also handle a state where a manifest could not be downloaded in self.create_listed_manifest?
There was a problem hiding this comment.
Let's keep refactoring and features separate.
Remove content interrelate stage in favor of content.resolution and _post_save. fixes pulp#495 Co-authored-by: Ľuboš Mjachky <lmjachky@redhat.com>
There was a problem hiding this comment.
I read through it and it exactly implements this pseudo code #495 (comment) except for the popping the dc and replacing with the resolved content, which is ok and i think even reads better simply by accessing the dc.content instead.
fixes #495