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

[reposync] Add latest NEVRAs per stream to download (RhBug: 1833074) #407

Merged
merged 1 commit into from Aug 5, 2020

Conversation

m-blaha
Copy link
Member

@m-blaha m-blaha commented Jul 23, 2020

This covers situation when package with the newest NEVRA is part of
an older version of a stream.

https://bugzilla.redhat.com/show_bug.cgi?id=1833074

Test: rpm-software-management/ci-dnf-stack#875

stream_artifacts.update(module.getArtifacts())
# find versions to which the packages with the highest NEVRAs belong
for lp in query.filter(nevra_strict=stream_artifacts).latest():
#XXX contains modules.yaml allways full NEVRA (including epoch?)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am sorry but I do not understand xxx note.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, the thing is that I need to pair package objects to artefacts as they are written in modules.yaml (artifacts are keys of the artifact_version dictionary). Currently the code relies on having artifacts in form of full NEVRA. I was just wondering if there is possibility that artifacts in modules.yaml are in a different format (e.g. zero epoch being left out).

for lp in query.filter(nevra_strict=stream_artifacts).latest():
#XXX contains modules.yaml allways full NEVRA (including epoch?)
nevra = "{0.name}-{0.epoch}:{0.version}-{0.release}.{0.arch}".format(lp)
versions.update(artifact_version[nevra][namestream])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not 100 % sure, but what happen in case when the latest artefact is present in all or multiple versions? Somehow I feel that in this case all versions will be downloaded. I suggest that in this case only the latest context should be used.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, currently all versions with the latest artefact versions are going to be downloaded. I'll try to limit it to the latest version in this case.

for module in modules:
stream_artifacts.update(module.getArtifacts())
# find versions to which the packages with the highest NEVRAs belong
for lp in query.filter(nevra_strict=stream_artifacts).latest():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about to rename lp to pkg or latest_pkg?

versions.add(max(artifact_version[nevra][namestream]))
# add all artifacts from selected versions for synchronization
for version, modules in version_dict.items():
if version in versions:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This not a problem, but anyway, the code could use a different logic.

for version in versions:
    for module in version_dict[version]:

@j-mracek
Copy link
Member

The logic and code LGTM!

This covers situation when package with the newest NEVRA is part of
an older version of a stream and reposync was used with --newest-only
switch.
With this patch these package versions are going to be downloaded:
- the latest NEVRAs from non-modular packages
- all packages from stream version with the latest package NEVRA (in
  case the latest NEVRA is part of multiple stream versions only the
  highest is downloaded)
- all packages from the latest stream version

https://bugzilla.redhat.com/show_bug.cgi?id=1833074
@m-blaha
Copy link
Member Author

m-blaha commented Jul 31, 2020

Thanks Jardo, I squashed all the commits into one (they were just unnecessary development noise) and fixed issues you found in it.

@j-mracek
Copy link
Member

j-mracek commented Aug 3, 2020

@rh-atomic-bot try

@rh-atomic-bot
Copy link

⌛ Trying commit 37d60b6 with merge 0c8b540...

rh-atomic-bot pushed a commit that referenced this pull request Aug 3, 2020
This covers situation when package with the newest NEVRA is part of
an older version of a stream and reposync was used with --newest-only
switch.
With this patch these package versions are going to be downloaded:
- the latest NEVRAs from non-modular packages
- all packages from stream version with the latest package NEVRA (in
  case the latest NEVRA is part of multiple stream versions only the
  highest is downloaded)
- all packages from the latest stream version

https://bugzilla.redhat.com/show_bug.cgi?id=1833074

Closes: #407
Approved by: <try>
@rh-atomic-bot
Copy link

☀️ Test successful - status-papr
State: approved= try=True

@j-mracek
Copy link
Member

j-mracek commented Aug 5, 2020

@rh-atomic-bot r+

@rh-atomic-bot
Copy link

📌 Commit 37d60b6 has been approved by j-mracek

@m-blaha
Copy link
Member Author

m-blaha commented Aug 5, 2020

I suspect that @rh-atomic-bot r+ does not work correctly after @rh-atomic-bot try. Merging manually.

@m-blaha m-blaha merged commit 5f54afb into master Aug 5, 2020
@m-blaha m-blaha deleted the mblaha/reposync-latest branch August 5, 2020 08:59
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