published updateinfo only contains units in repo #825
Conversation
|
The unit tests written for this case do a reasonable job of making sure that the integration points are being called correctly, and that the new code does the right thing with expected return values from those integration points, but a "real" integration test is needed to properly cover this. I've opened a pulp-smash issue to track that effort: pulp/pulp-smash#163 |
| repo_unit_nevra = [] | ||
| for nevra_unit_id, nevra_field_values in nevra_unit_map.items(): | ||
| # based on the args to scalar when nevra_units was created: | ||
| if nevra_unit_id in repo_unit_ids: |
There was a problem hiding this comment.
In case repo_unit_ids has many entries, it would perform this operation better as a set.
There was a problem hiding this comment.
The magnitude of repo_unit_ids length is on the order of dozens to hundreds which is trivial for a list to do well, but on this scale a set will likely always do better.
There was a problem hiding this comment.
At least for me, the diff related to this comment thread is out of date. repo_unit_ids is now a set, which can be seen in the diff view.
There was a problem hiding this comment.
ack, I see that too now. Not sure if my view was out of date or if I just missed it.
| @@ -102,6 +164,16 @@ def add_unit_metadata(self, item): | |||
| 'epoch': package['epoch'] or '0', | |||
| 'arch': package['arch'], | |||
| 'src': package.get('src', '') or ''} | |||
|
|
|||
| if repo_unit_nevra is not None: | |||
There was a problem hiding this comment.
I think this is a reasonable approach given the way this function is setup. But just to say it, in an ideal world it would be better if there were a way to move this logic outside the function. Then add_unit_metadata could focus on adding metadata, and not on deciding which data to add. The obvious design would be to filter or similar up on line 159, but because of the transformation that happens in between there and here, that isn't convenient. without changing lots of stuff.
Oh well. Just an observation.
|
Looks good. Thanks for the thorough in-line documentation. |
Errata units in Pulp contain all units in all repos that are linked to errata with the same id, which was resulting in published errata referencing packages that weren't actually available in the published repo. This limits packages in published errata updateinfo XML to only the packages that are contained in the published repo. fixes pulp#1366 https://pulp.plan.io/issues/1366 fixes pulp#1548 https://pulp.plan.io/issues/1548
|
ok test |
published updateinfo only contains units in repo
Errata units in Pulp contain all units in all repos that are linked to
errata with the same id, which was resulting in published errata
referencing packages that weren't actually available in the published
repo. This limits packages in published errata updateinfo XML to only
the packages that are contained in the published repo.
fixes #1366
https://pulp.plan.io/issues/1366
fixes #1548
https://pulp.plan.io/issues/1548