fix(gitea): prevent scheduling on not-supported architectures#425
Draft
okurz wants to merge 10 commits intoopenSUSE:masterfrom
Draft
fix(gitea): prevent scheduling on not-supported architectures#425okurz wants to merge 10 commits intoopenSUSE:masterfrom
okurz wants to merge 10 commits intoopenSUSE:masterfrom
Conversation
af5de2a to
202a56a
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #425 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 85 85
Lines 8202 8286 +84
Branches 443 450 +7
=========================================
+ Hits 8202 8286 +84 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This comment was marked as resolved.
This comment was marked as resolved.
asmorodskyi
reviewed
Mar 6, 2026
|
|
||
| def verify_repo_exists(project: str, product_name: str, product_version: str, arch: str) -> bool: | ||
| """Check if the repository actually exists for the given architecture via HTTP HEAD request.""" | ||
| if not product_version: |
Contributor
There was a problem hiding this comment.
can you elaborate why when product_version is missing we actually returning True ? I can understand False or exception but True looks strange
asmorodskyi
reviewed
Mar 6, 2026
openqabot/loader/gitea.py
Outdated
| return True | ||
| download_base = config.settings.download_base_url | ||
| obs_download = config.settings.obs_download_url | ||
| if not download_base or not obs_download: |
Contributor
There was a problem hiding this comment.
how this suppose to work ? this variables has default values so isn't they ALWAYS has some value ?
Member
Author
|
marked as draft to address feedback from others about packages which only publish something in "noarch" |
asmorodskyi
reviewed
Mar 6, 2026
openqabot/loader/gitea.py
Outdated
| obs_download = config.settings.obs_download_url | ||
| if not download_base or not obs_download: | ||
| return True | ||
| try: |
Contributor
There was a problem hiding this comment.
Suggested change
| try: | |
| parts = obs_download.split("/") | |
| if len(parts) < 2: | |
| return True | |
| host = parts[2] |
3812d1d to
6536108
Compare
6536108 to
b1a531f
Compare
Motivation:
OBS returns state='published' with code='excluded' for packages on
architectures where no builds exist. This caused qem-bot to create
channels for unsupported architectures (e.g., ppc64le, s390x for
SLES 16.0), leading to openQA job failures when the repo subfolder
doesn't actually exist.
This change verifies that actual repository content exists via HTTP
requests before creating channel.
Design Choices:
- Added _verify_repo_exists() function that performs an HTTP HEAD
request to check if the architecture-specific repo folder exists
- Check happens in add_channel_for_build_result() before adding to
the projects set
- If HTTP returns 404, the channel is skipped
- Uses download.suse.de (replacing %REPO_MIRROR_HOST% placeholder)
to check actual repo availability
- Fails open: if HTTP check fails for any reason, allow the channel
User Benefits:
- Prevents scheduling of tests on non-supported architectures
- Based on actual repo existence, not OBS package status
- Maintains backward compatibility for existing valid channels
For a specific example of SUSE internal PR "git:2702" a run of `qem-bot
gitea-sync` looks like this showing how channels for all architectures
are considered even the ones for which repository arch specific
subfolders are empty:
```
INFO Loaded 1 active PRs from products/SLFO
DEBUG Fetching info for PR git:2702 from Gitea
DEBUG Checking OBS project SUSE:SLFO:1.2:PullRequest:2702
DEBUG Checking OBS project SUSE:SLFO:1.2:PullRequest:2702:SL-Micro
DEBUG Relevant archs for SUSE:SLFO:1.2:PullRequest:2702:SL-Micro: \
['aarch64', 'ppc64le', 's390x', 'x86_64']
DEBUG Checking OBS project SUSE:SLFO:1.2:PullRequest:2702:SLES
DEBUG Relevant archs for SUSE:SLFO:1.2:PullRequest:2702:SLES: \
['aarch64', 'ppc64le', 's390x', 'x86_64']
DEBUG Data for 1 submissions: [{'approved': True,
'channels': ['SUSE:SLFO:1.2:PullRequest:2702:SL-Micro:aarch64#6.2',
'SUSE:SLFO:1.2:PullRequest:2702:SL-Micro:ppc64le#6.2',
'SUSE:SLFO:1.2:PullRequest:2702:SL-Micro:s390x#6.2',
'SUSE:SLFO:1.2:PullRequest:2702:SL-Micro:x86_64#6.2',
'SUSE:SLFO:1.2:PullRequest:2702:SLES:aarch64#16.0',
'SUSE:SLFO:1.2:PullRequest:2702:SLES:ppc64le#16.0',
'SUSE:SLFO:1.2:PullRequest:2702:SLES:s390x#16.0',
'SUSE:SLFO:1.2:PullRequest:2702:SLES:x86_64#16.0'],
…
'type': 'git',
'url': 'https://src.suse.de/products/SLFO/pulls/2702'}]
INFO Dry run: Would update QEM Dashboard data for 1 submissions
```
so also invalid channels like
SUSE:SLFO:1.2:PullRequest:2702:SLES:ppc64le#16.0 where the patch does
not exist are added.
With this commit repository folders are probed and channels adjusted
accordingly:
```
INFO Loaded 1 active PRs from products/SLFO
DEBUG Fetching info for PR git:2702 from Gitea
DEBUG Checking OBS project SUSE:SLFO:1.2:PullRequest:2702
DEBUG Checking OBS project SUSE:SLFO:1.2:PullRequest:2702:SL-Micro
DEBUG Relevant archs for SUSE:SLFO:1.2:PullRequest:2702:SL-Micro: \
['aarch64', 'ppc64le', 's390x', 'x86_64']
DEBUG Repo http://…/SL-Micro-6.2-x86_64/x86_64/ returned 200, allowing channel
DEBUG Repo http://…/SL-Micro-6.2-aarch64/aarch64/ returned 200, allowing channel
DEBUG Repo http://…-ppc64le/ppc64le/ not found (404), skipping channel
DEBUG Skipping …:SL-Micro:ppc64le: repository not found for architecture ppc64le
DEBUG Repo http://…-s390x/s390x/ not found (404), skipping channel
DEBUG Skipping …:SL-Micro:s390x: repository not found for architecture s390x
DEBUG Checking OBS project SUSE:SLFO:1.2:PullRequest:2702:SLES
DEBUG Relevant archs for SUSE:SLFO:1.2:PullRequest:2702:SLES: \
['aarch64', 'ppc64le', 's390x', 'x86_64']
DEBUG Repo http://…/SLES-16.0-aarch64/aarch64/ returned 200, allowing channel
DEBUG Repo http://…/SLES-16.0-ppc64le/ppc64le/ not found (404), skipping channel
DEBUG Skipping …:SLES:ppc64le: repository not found for architecture ppc64le
DEBUG Repo http://…/SLES-16.0-s390x/s390x/ not found (404), skipping channel
DEBUG Skipping …:SLES:s390x: repository not found for architecture s390x
DEBUG Repo http://…/SLES-16.0-x86_64/x86_64/ returned 200, allowing channel
DEBUG Data for 1 submissions: [{'approved': True,
'channels': ['SUSE:SLFO:1.2:PullRequest:2702:SL-Micro:aarch64#6.2',
'SUSE:SLFO:1.2:PullRequest:2702:SL-Micro:x86_64#6.2',
'SUSE:SLFO:1.2:PullRequest:2702:SLES:aarch64#16.0',
'SUSE:SLFO:1.2:PullRequest:2702:SLES:x86_64#16.0'],
'type': 'git',
'url': 'https://src.suse.de/products/SLFO/pulls/2702'}]
INFO Dry run: Would update QEM Dashboard data for 1 submissions
```
with the relevant part being
```
'channels': ['SUSE:SLFO:1.2:PullRequest:2702:SL-Micro:aarch64#6.2',
'SUSE:SLFO:1.2:PullRequest:2702:SL-Micro:x86_64#6.2',
'SUSE:SLFO:1.2:PullRequest:2702:SLES:aarch64#16.0',
'SUSE:SLFO:1.2:PullRequest:2702:SLES:x86_64#16.0'],
```
so only aarch64+x86_64 channels are included as expected.
Related progress issue: https://progress.opensuse.org/issues/197270
Motivation: Manual slicing of URLs is brittle and can lead to errors if the URL format changes or is unexpected. Design Choices: - Used urllib.parse.urlparse to safely extract the netloc (hostname) from the OBS download URL. - Added explicit aliases for osc submodules (osc_conf, osc_core, osc_xml) to satisfy type checking and improve clarity. - Added # noqa: F401 to import osc to satisfy ruff while keeping the submodules available as attributes if needed (though now mostly using explicit aliases). User Benefits: - Improved robustness of repository existence verification. - Better maintainability through explicit module usage.
Motivation: Repo URL construction and project path transformation were duplicated or hardcoded in the loader, making it hard to maintain and test. Design Choices: - Added get_repo_url to openqabot/utils.py to centralize repository URL generation. - Used keyword-only arguments for environmental parameters to maintain clarity despite many arguments. - Added explicit imports for osc submodules in openqabot/loader/gitea.py to satisfy type checking and satisfy the code health checks. User Benefits: - Improved maintainability through centralized logic. - Easier testing of repository URL construction in isolation.
Motivation: Functions relying on global configuration are harder to test and maintain. Decoupling them by passing parameters improves modularity and testability. Design Choices: - Updated verify_repo_exists, add_channel_for_build_result, and get_product_version_from_repo_listing to accept configuration parameters as arguments instead of accessing config.settings directly. - Used keyword-only arguments for environmental parameters to maintain clarity and satisfy linting rules (PLR0913, PLR0917). - Updated all call sites and tests to reflect the new function signatures. User Benefits: - Improved architectural cleanliness and maintainability. - More robust and isolated unit tests.
Motivation: Redundant logging and brittle error handling can obscure actual issues and make the code harder to maintain. Design Choices: - Consolidated logging in verify_repo_exists to avoid redundancy with its callers. - Promoted debug logs to info for HTTP failures to provide better visibility in fail-open scenarios. - Removed redundant log message from add_channel_for_build_result. - Updated tests to reflect the new logging behavior and fixed unused function arguments flagged by ruff. User Benefits: - Cleaner log output. - More informative logs when repository verification encounters unexpected network issues.
Motivation: Functions with too many arguments are difficult to maintain and understand. Grouping related parameters into dataclasses improves code structure and satisfies linting rules. Design Choices: - Introduced BuildTarget dataclass to group project, arch, and product_name. - Introduced RepoConfig dataclass to group repository-related settings. - Refactored get_repo_url, verify_repo_exists, and add_channel_for_build_result to use these new dataclasses. - Removed all PLR0913 and PLR0917 exclusions. - Updated all call sites and tests to reflect the new structure. User Benefits: - Improved code readability and maintainability. - More robust and cleaner function signatures.
Motivation: Placing logic-agnostic data structures in utils.py is architecturally inconsistent. Moving them to a dedicated types module improves project structure and maintainability. Design Choices: - Created openqabot/types/gitea.py for Gitea-specific type definitions. - Relocated BuildTarget and RepoConfig dataclasses to the new module. - Updated all call sites, tests, and utility functions to use the new location. - Utilized TYPE_CHECKING blocks where appropriate to prevent circular imports and improve performance. User Benefits: - Better organized codebase. - More predictable location for data structure definitions.
Motivation: Generic docstrings for domain-specific data structures can be ambiguous. Explicitly mentioning OBS repository mirrors and build targets improves code clarity. Design Choices: - Updated BuildTarget docstring to explicitly mention OBS and its components. - Refined RepoConfig docstring to specify its role in OBS repository mirror configuration. User Benefits: - Improved developer documentation. - Reduced ambiguity regarding the purpose of these configuration structures.
Motivation: Maintaining multiple identical structures for repository coordinates (project, arch, product) leads to technical debt and structural duplication. Design Choices: - Replaced the short-lived BuildTarget dataclass with the established Repos NamedTuple. - Mapped project to Repos.product and product_name to Repos.version to align with existing SLFO patterns in the codebase. - Simplified add_channel_for_build_result return logic to satisfy PLR0911 while maintaining functional correctness. - Updated all call sites and tests to utilize the Repos type. User Benefits: - Reduced code duplication and structural complexity. - Improved architectural consistency across the project.
Motivation: Validating configuration at runtime is brittle and can lead to silent fail-open failures. Moving validation to the configuration layer ensures that the application starts with a known-good state. Design Choices: - Added a field_validator to Settings for obs_download_url to ensure it contains a valid hostname. - Introduced a repo_mirror_host property in Settings for centralized and safe hostname extraction. - Refactored RepoConfig and get_repo_url to use the pre-validated repo_mirror_host. - Removed redundant runtime checks and suppressed exceptions in verify_repo_exists. - Added comprehensive tests for the new validator and property in tests/test_config.py. User Benefits: - Faster failure on malformed configuration. - More robust and predictable repository verification logic.
b1a531f to
1b1710a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation:
OBS returns state='published' with code='excluded' for packages on
architectures where no builds exist. This caused qem-bot to create
channels for unsupported architectures (e.g., ppc64le, s390x for
SLES 16.0), leading to openQA job failures when the repo subfolder
doesn't actually exist.
This change verifies that actual repository content exists via HTTP
requests before creating channel.
Design Choices:
request to check if the architecture-specific repo folder exists
the projects set
to check actual repo availability
User Benefits:
For a specific example of SUSE internal PR "git:2702" a run of
qem-bot gitea-synclooks like this showing how channels for all architecturesare considered even the ones for which repository arch specific
subfolders are empty:
so also invalid channels like
SUSE:SLFO:1.2:PullRequest:2702:SLES:ppc64le#16.0 where the patch does
not exist are added.
With this commit repository folders are probed and channels adjusted
accordingly:
with the relevant part being
so only aarch64+x86_64 channels are included as expected.
Related progress issue: https://progress.opensuse.org/issues/197270