[6.17.z] Use Convert2RHEL repofiles instead of deprecated repos for tests - #20889
Merged
Satellite-QE merged 1 commit intoFeb 26, 2026
Merged
Conversation
) Signed-off-by: Gaurav Talreja <gtalreja@redhat.com> (cherry picked from commit 0644d11)
Satellite-QE
added
6.17.z
Auto_Cherry_Picked
Collaborator
Author
|
Satellite-QE
added
the
AutoMerge_Cherry_Picked
Contributor
Reviewer's GuideUpdate Convert2RHEL API tests to obtain repository base URLs by downloading and parsing remote .repo files, replacing use of deprecated hard-coded repository URLs, and add a shared helper for extracting baseurls from repofiles. Sequence diagram for get_baseurl_by_repofile resolving baseurlsequenceDiagram
participant TestCase
participant ContentInfo
participant Requests
participant RepoServer
TestCase->>ContentInfo: get_baseurl_by_repofile(repo_url, verify_ssl)
ContentInfo->>Requests: get(repo_url, verify_ssl, timeout=10)
Requests->>RepoServer: HTTP GET repo_url
RepoServer-->>Requests: 200 OK, .repo content
Requests-->>ContentInfo: response(text)
ContentInfo->>ContentInfo: parse lines, find baseurl=
ContentInfo-->>TestCase: baseurl string
Class diagram for updated content_info utilitiesclassDiagram
class ContentInfo {
+get_repo_files_by_url(url, extension)
+get_repo_files_urls_by_url(url, extension)
+get_repomd(repo_url)
+get_baseurl_by_repofile(repo_url, verify_ssl)
}
class Requests {
+get(url, verify, timeout)
}
ContentInfo ..> Requests : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new
get_baseurl_by_repofileparser is quite strict (line.startswith('baseurl=')), so it will miss common variations likebaseurl = ...or uppercase keys; consider usingconfigparseror a more flexible parse (e.g., splitting on=after checkingline.lower().startswith('baseurl')). - Instead of hard-coding the
timeout=10inrequests.get, consider reusing any existing HTTP helper or making the timeout configurable/centralized to keep network behavior consistent across the module.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `get_baseurl_by_repofile` parser is quite strict (`line.startswith('baseurl=')`), so it will miss common variations like `baseurl = ...` or uppercase keys; consider using `configparser` or a more flexible parse (e.g., splitting on `=` after checking `line.lower().startswith('baseurl')`).
- Instead of hard-coding the `timeout=10` in `requests.get`, consider reusing any existing HTTP helper or making the timeout configurable/centralized to keep network behavior consistent across the module.
## Individual Comments
### Comment 1
<location path="robottelo/content_info.py" line_range="81-90" />
<code_context>
+ for line in response.text.splitlines():
+ line = line.strip()
+
+ if line.startswith('baseurl='):
+ return line.split('=', 1)[1].strip()
+
+ raise ValueError(f'No baseurl found in {repo_url}')
</code_context>
<issue_to_address>
**suggestion:** Handle inline comments and empty values when extracting the baseurl
This will return anything after `baseurl=`, including inline comments (e.g. `baseurl=http://foo # comment`) or an empty value. Consider stripping inline comments first (e.g. split on `#`), ensuring the value is non-empty and URL-like, and otherwise continuing the scan or raising a clearer error to avoid silently returning malformed values.
```suggestion
response = requests.get(repo_url, verify=verify_ssl, timeout=10)
response.raise_for_status()
for line in response.text.splitlines():
line = line.strip()
if line.startswith('baseurl='):
# Extract raw value after "baseurl="
raw_value = line.split('=', 1)[1].strip()
if not raw_value:
# Empty baseurl, continue scanning other lines
continue
# Strip inline comments: everything after '#' is considered a comment
value = raw_value.split('#', 1)[0].strip()
if not value:
# Only comment or whitespace after baseurl=, keep scanning
continue
# Basic sanity check to avoid obviously malformed values
# (most yum/dnf baseurls are URLs or file paths)
if "://" not in value and not value.startswith("file:"):
# Not URL-like, keep scanning for a better candidate
continue
return value
raise ValueError(f'No valid baseurl found in repository metadata from {repo_url}')
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+81
to
+90
| response = requests.get(repo_url, verify=verify_ssl, timeout=10) | ||
| response.raise_for_status() | ||
|
|
||
| for line in response.text.splitlines(): | ||
| line = line.strip() | ||
|
|
||
| if line.startswith('baseurl='): | ||
| return line.split('=', 1)[1].strip() | ||
|
|
||
| raise ValueError(f'No baseurl found in {repo_url}') |
Contributor
There was a problem hiding this comment.
suggestion: Handle inline comments and empty values when extracting the baseurl
This will return anything after baseurl=, including inline comments (e.g. baseurl=http://foo # comment) or an empty value. Consider stripping inline comments first (e.g. split on #), ensuring the value is non-empty and URL-like, and otherwise continuing the scan or raising a clearer error to avoid silently returning malformed values.
Suggested change
| response = requests.get(repo_url, verify=verify_ssl, timeout=10) | |
| response.raise_for_status() | |
| for line in response.text.splitlines(): | |
| line = line.strip() | |
| if line.startswith('baseurl='): | |
| return line.split('=', 1)[1].strip() | |
| raise ValueError(f'No baseurl found in {repo_url}') | |
| response = requests.get(repo_url, verify=verify_ssl, timeout=10) | |
| response.raise_for_status() | |
| for line in response.text.splitlines(): | |
| line = line.strip() | |
| if line.startswith('baseurl='): | |
| # Extract raw value after "baseurl=" | |
| raw_value = line.split('=', 1)[1].strip() | |
| if not raw_value: | |
| # Empty baseurl, continue scanning other lines | |
| continue | |
| # Strip inline comments: everything after '#' is considered a comment | |
| value = raw_value.split('#', 1)[0].strip() | |
| if not value: | |
| # Only comment or whitespace after baseurl=, keep scanning | |
| continue | |
| # Basic sanity check to avoid obviously malformed values | |
| # (most yum/dnf baseurls are URLs or file paths) | |
| if "://" not in value and not value.startswith("file:"): | |
| # Not URL-like, keep scanning for a better candidate | |
| continue | |
| return value | |
| raise ValueError(f'No valid baseurl found in repository metadata from {repo_url}') |
Collaborator
Author
|
PRT Result |
Satellite-QE
added
the
PRT-Passed
Satellite-QE
deleted the
cherry-pick-6.17.z-0644d11f807200c8014d37d1ac667fccd0952529
branch
February 26, 2026 14:00
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
2 participants
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.
Cherrypick of PR: #20869
Problem Statement
Solution
Related Issues
Summary by Sourcery
Update Convert2RHEL API tests to use base URLs derived from remote repofiles instead of hard-coded repository URLs.
Bug Fixes:
Enhancements: