Detect and error on duplicate artifact NVRAs

[AthreyVinay]

from sync...
Specific example:
User provides two scratch builds of curl
Two koji tasks with the same nvr (content is different, e.g. two ways / patches how to fix the bug)
Raise an error (as we cannot decide/check what should be installed)

NOTE: This pr also renamed raw_provider_id -> raw_id.

• extend tests

[gemini-code-assist]

Summary of Changes

Hello @AthreyVinay, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the robustness of the artifact preparation step by introducing validation for artifact NVRAs. It ensures that if multiple artifact providers attempt to supply artifacts with identical NVRAs, an explicit error is raised. This prevents potential conflicts and ensures a clear, deterministic source for each artifact, thereby improving the overall reliability and predictability of the artifact management process.

Highlights

• Duplicate NVRA Detection: Implemented a mechanism to detect and error on duplicate artifact NVRAs (Name-Version-Release-Architecture) provided by different sources, preventing ambiguous artifact installations.
• Error Handling: Introduced a PrepareError to be raised when duplicate NVRAs are found, ensuring that the process halts and notifies the user of the conflict.
• Code Refactoring: Refactored the artifact metadata collection and duplicate checking logic into a new dedicated private method, _collect_artifacts_metadata, improving modularity and readability.

Changelog

• tmt/steps/prepare/artifact/init.py
  • Updated specific type imports from the typing module for Any, ClassVar, and Optional.
  • Introduced seen_nvras and providers_data variables within the go method to track artifact NVRAs and collected metadata.
  • Extracted artifact metadata collection and duplicate NVRA detection into a new private method _collect_artifacts_metadata.
  • Modified the _persist_artifact_metadata method to accept pre-collected artifact data, streamlining its responsibility.
  • Integrated the new _collect_artifacts_metadata method into the artifact processing loop to perform duplicate checks and gather data.

Activity

• No specific activity has been recorded for this pull request yet.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request effectively addresses the requirement to detect and error on duplicate artifact NVRAs by introducing the _collect_artifacts_metadata method and refactoring _persist_artifact_metadata. No security vulnerabilities were found, and the changes are well-implemented, directly fulfilling the stated objective.

[tcornell-bus]

Hi @AthreyVinay looks like there are some conflicts. Could you take a look please?

[AthreyVinay]

Could you take a look please?

Thanks @tcornell-bus , done.

[AthreyVinay]

I think the user would prefer it to fail quickly, rather than halfway down the download list.

Thank you @tcornell-bus, makes sense to me too.
Addressed here: Move duplicate NVRA detection into a separate pass that runs...

[tcornell-bus]

Should this also handle when there are two different repository-files that provide the same package NVRA? Or is this only to detect when koji:<task-id-1> and koji:<task-id-2> point to the same NVRA?

[happz]

Should this also handle when there are two different repository-files that provide the same package NVRA? Or is this only to detect when koji:<task-id-1> and koji:<task-id-2> point to the same NVRA?

As of now, I'd focus on non-repository providers like koji and brew. Providers contributing their payload to the shared repository. Repositories brought by repository-like providers may be disabled except in one particular test, or mapped into VMs, plenty of ways for a potential NVR conflict to never materialize; on the other hand, package-like providers contributing same NVR to the same repository is broken already.

[LecrisUT]

Fine overall, just some minor code style comments

[AthreyVinay]

As of now, I'd focus on non-repository providers like koji and brew. Providers contributing their payload to the shared repository

Maybe an explicit check before we detect duplcates? .... something along the lines of:

if provider.contribute_to_shared_repo();
    self._detect_duplicate_nvras()

[LecrisUT]

As of now, I'd focus on non-repository providers like koji and brew. Providers contributing their payload to the shared repository

Maybe an explicit check before we detect duplcates? .... something along the lines of:

if provider.contribute_to_shared_repo();
    self._detect_duplicate_nvras()

Fine either way. If there are no .artifacts provided then we do not have anything to use the duplicate nvr check for. Although the plain name of .artifacts for that makes things increasingly confusing, since we are converging to using that property to mean installable_artifacts more explicitly (we do not query or populate that for repo file, and I don't think we should start to do either)

[AthreyVinay]

Although the plain name of .artifacts for that makes things increasingly confusing, since we are converging to using that property to mean installable_artifacts more explicitly

Agree - there was some discussion around it in a pr here: #4526 (comment)