supervisor: Patch get_previous_erratum logic#372
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the get_previous_erratum function to handle an edge case where an erratum ID from the API can be null. The function now returns the erratum ID and build NVR as a tuple, deferring the decision to fetch the full Erratum object to the caller. The changes are correctly propagated to the call sites in erratum_handler.py and issue_handler.py.
My review includes a couple of suggestions to improve maintainability and readability. One suggestion is to refactor a block of code in errata_utils.py for conciseness and to use safer dictionary access. Another suggestion is to centralize the construction of errata advisory URLs to avoid hardcoding them in multiple places.
owtaylor
left a comment
owtaylor
left a comment
There was a problem hiding this comment.
The basic idea here is right, but some changes are needed to make it actually do what is intended - handle the case where we have a build nvr but no errata.
Jazzcort
force-pushed
the
patch-previous-erratum-logic
branch
2 times, most recently
from
1fc9f13 to
d6e8d16
Compare
supervisor/errata_utils.py
Outdated
| nvr, | ||
| ) | ||
| if nvr is not None | ||
| else (None, None) |
There was a problem hiding this comment.
I think I'd rather do:
if nvr is None:
raise RuntimeException(f"{latest_erratum.id}, returned by Errata tool as an errata for {package_name}, does not have a build for {package_name}")
or something like that - this isn't supposed to happen, if it does happen, we need to figure out why it is happening and what the appropriate handling is here.
There was a problem hiding this comment.
Yeah, I agree! It's better to raise an error here rather than a silent pass. will change! 😁
This function used to fetch the Erratum object after it found the previous
erratum. However, we encountered an edge case that the erratum_id from the
erratatools API's response of
<endpoint>/api/v1/product_versions/{RHEL version}/released_builds/{package name}
cound be null in REHL 8. To address this, we decide to make this function just
return the erratum id and build nvr so we can decide whether we should fetch the
Erratum object or not after calling this function.
Also adjust the logic where this function is called.
Adds get_erratum_advisory_url, removes trailing slash from ET_URL, and updates erratum URL construction across the codebase.
Jazzcort
force-pushed
the
patch-previous-erratum-logic
branch
from
d6e8d16 to
0fce4ae
Compare
3 participants
This function used to fetch the Erratum object after it found the previous erratum. However, we encountered an edge case that the erratum_id from the erratatools API's response of
/api/v1/product_versions/{RHEL version}/released_builds/{package name}
cound be null in REHL 8. To address this, we decide to make this function just return the erratum id and build nvr so we can decide whether we should fetch the Erratum object or not after calling this function.
Also adjust the logic where this function is called.