Simplify re-raise for non-legacy

Glutexo commented 1 month ago

All Pull Requests:

Check all that apply:

[x] Have you followed the guidelines in our Contributing document, including the instructions about commit messages?
[x] No Sensitive Data in this change?
[x] Is this PR to correct an issue?
[ ] Is this PR an enhancement?

Complete Description of Additions/Changes:

To avoid confusion for developers, removed the unnecessary conditional for exception re-raise.

It is unnecessary to set last_ex on non-legacy, because there is no fallback to other URLs. Only one URL is tried and if it fails, there is only a single exception.
The if last_ex condition can never be false, because if an exception is not raised, the function hits a return clause earlier. It only gets to the conditional if an exception has been caught.

A simple raise is enough. Without an argument, the last caught exception is re-raised by default. It’s thus equivalent to raise exc.

Card IDs:

CCT-867

codecov-commenter commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 77.18%. Comparing base (4280fd0) to head (775c63f).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #4254 +/- ## ======================================= Coverage 77.18% 77.18% ======================================= Files 762 762 Lines 41478 41476 -2 Branches 8764 8763 -1 ======================================= + Hits 32014 32015 +1 + Misses 8407 8406 -1 + Partials 1057 1055 -2 ``` | [Flag](https://app.codecov.io/gh/RedHatInsights/insights-core/pull/4254/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=RedHatInsights) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/RedHatInsights/insights-core/pull/4254/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=RedHatInsights) | `77.17% <100.00%> (+<0.01%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=RedHatInsights#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Glutexo commented 3 weeks ago

Rebased on the current master. Resolved conflicts. Ready for review.

Glutexo commented 3 weeks ago

Removed the unrelated reformatting. Ready for re-review/merge.

What about reformatting the codebase with Black? Then, we could add it to the CI and reject non-conforming code. JIRA card?

xiangce commented 3 weeks ago

What about reformatting the codebase with Black?

I'm generally in favor, but 1) we don't own the repository, 2) I'm afraid Black/ruff may introduce formatting that's not compatible with Python 2.6/2.7. I think it should be on a long-term roadmap, but there are more difficult problems to be solved first. Yup, I have ever tried to enable black. You know, the core project was open-sourced a long time but without such code formaters. It would make tiny updates caused a lot of irrelevant changes.

RedHatInsights / insights-core