Conversation
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| ErrorProne | 9 medium 1 high |
| Security | 20 high |
🟢 Metrics 35 complexity · 0 duplication
Metric Results Complexity 35 Duplication 0
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes. Give us feedback
There was a problem hiding this comment.
Code Review
This pull request introduces support for Red Hat Enterprise Linux (RHEL) by adding a new data file, a downloader script that fetches lifecycle information from the Red Hat API, and corresponding tests. Feedback suggests improving the robustness of the API parser by handling empty responses and using more appropriate exception types. Additionally, it is recommended to rename generic variables in the update tool and ensure deterministic JSON output by sorting keys.
Fetches lifecycle data from the official Red Hat Product Life Cycle Data API (access.redhat.com/product-life-cycles/api/v1). Maps phases as: - begin_support: General availability date - end_support: End of Maintenance support - end_extended_support: End of ELS add-on begin_dev is set equal to begin_support so that is_in_development_on always returns False (Red Hat does not publish pre-GA development dates). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Use json.load() instead of json.loads(response.read()) for efficiency - Replace ConnectionError with RuntimeError for HTTP error status - Guard against empty data[] response to avoid IndexError - Use .get() for 'versions' and 'phases' for schema robustness - Calculate GA date once and reuse for begin_support and begin_dev - Rename ubuntu_data variable in update.py to data_path (generic) - Add sort_keys=True to json.dumps for deterministic output order - Re-sort existing ubuntu and debian JSON data files accordingly Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The URL is a hardcoded constant, not user input, so the audit warning does not apply. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Sort versions numerically (not lexicographically) so 4.10 < 10.04 - Empty-string version (Debian sid) sorts last, preserving its position - Drop sort_keys=True which was reordering inner keys alphabetically; insertion order from get_distro_info() already provides the right order Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…("inf")
float("inf") returns tuple[float] which doesn't match the tuple[int, ...]
return type annotation. sys.maxsize achieves the same "sort last" effect
while keeping the type consistent.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds Red Hat Enterprise Linux (RHEL) support to distro-support, including online fetching from Red Hat’s lifecycle API plus bundled offline JSON data, and extends the test suite to cover the new distro.
Changes:
- Add
src/distro_support/rhel.pyto fetch and map Red Hat lifecycle phase data into the project’s support-range schema. - Bundle offline lifecycle data in
src/distro_support/rhel.jsonand wire RHEL intotools/update.py. - Add dedicated downloader tests and extend
test_get_support_rangeintegration coverage for RHEL.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/update.py | Adds RHEL to the data regeneration tool and enforces version-key sorting when writing JSON. |
| tests/test_rhel_downloader.py | New unit tests for RHEL downloader behavior (date parsing, phase mapping, headers, HTTP errors). |
| tests/test_get_support_range.py | Adds RHEL scenarios to the integration-style support/ESM/dev assertions. |
| src/distro_support/rhel.py | Implements the RHEL online data fetch + mapping into JSON-compatible dicts. |
| src/distro_support/rhel.json | Bundled offline RHEL lifecycle dataset (versions 6–10). |
| req = request.Request(SUPPORT_INFO_URL, headers={"User-Agent": "distro-support"}) | ||
| with request.urlopen(req) as response: # nosec B310 |
There was a problem hiding this comment.
urlopen() is called without a timeout, so an unresponsive Red Hat API can hang indefinitely. The Alpine downloader uses an explicit timeout; consider adding a reasonable timeout here as well to avoid blocking callers when get_online=True.
| "begin_support": ga_date, | ||
| "end_support": _parse_date( | ||
| phases.get(_PHASE_MAINTENANCE, {}).get("end_date") | ||
| ), | ||
| "begin_dev": ga_date, | ||
| "end_extended_support": _parse_date( |
There was a problem hiding this comment.
The PR description notes begin_dev is set to the GA date so is_in_development_on always returns False, but if GA is missing (_parse_date() returns None for "N/A"/"Ongoing"), then begin_dev becomes None and SupportRange.is_in_development_on() will raise NoDevelopmentInfoError. Either ensure GA is always present for supported RHEL versions (so this invariant holds), or adjust the stated behavior/tests to reflect that development info may be unavailable when GA is missing.
| mock_response = MagicMock() | ||
| mock_response.status = 200 | ||
| mock_response.read.return_value = json.dumps(data).encode() | ||
| mock_response.__enter__ = lambda s: s |
There was a problem hiding this comment.
_make_mock_response() defines __enter__ as lambda s: s, but context managers call __enter__() with no arguments. This will raise TypeError when get_distro_info() executes with request.urlopen(...) as response:. Use a zero-arg __enter__ (or a MagicMock with return_value=mock_response) so the mocked response can be used in a with block.
| mock_response.__enter__ = lambda s: s | |
| mock_response.__enter__ = MagicMock(return_value=mock_response) |
| raise RuntimeError( | ||
| f"Unexpected HTTP status from Red Hat API: {response.status}" | ||
| ) |
There was a problem hiding this comment.
Other downloaders raise ConnectionError(response.status) for non-200 responses (e.g., alpine.py and _debian_like_downloader.py). For consistency (and easier reuse of error-handling/tests), consider raising ConnectionError(response.status) here instead of RuntimeError.
| raise RuntimeError( | |
| f"Unexpected HTTP status from Red Hat API: {response.status}" | |
| ) | |
| raise ConnectionError(response.status) |
Adds support for RHEL using the official Red Hat Product Life Cycle Data API.
Changes
rhel.pyfetches from the official Red Hat lifecycle API, mapping General availability →begin_support, Maintenance support end →end_support, and ELS add-on end →end_extended_supportrhel.jsonfor offline use (RHEL 6–10)test_rhel_downloader.py) covering date parsing, phase mapping, missing phases, User-Agent header, and HTTP errorstest_get_support_range.py