Page MenuHomeSoftware Heritage

BaseLoader: Fix handling of exceptions derived directly from BaseException
ClosedPublic

Authored by vlorentz on Fri, May 13, 11:08 AM.

Diff Detail

Repository
rDLDBASE Generic VCS/Package Loader
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

do not send SystemExit/KeyboardInterrupt to Sentry

Build is green

Patch application report for D7828 (id=28265)

Rebasing onto f510a00d10...

Current branch diff-target is up to date.
Changes applied before test
commit 055c6c0112878aacaf4439b23a7b7d6b816b1159
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri May 13 11:08:24 2022 +0200

    BaseLoader: Fix handling of exceptions derived directly from BaseException
    
    This reverts f510a00d10b3451161d3af756e415b9916d6d8fa to replace it
    with a fix of the underlying issue.

See https://jenkins.softwareheritage.org/job/DLDBASE/job/tests-on-diff/788/ for more details.

I think the BaseException change is sensible, but even though the new way of doing it is correct, initializing the success variable at the beginning of the try: block is the easiest way to reason about it.

Build is green

Patch application report for D7828 (id=28267)

Rebasing onto f510a00d10...

Current branch diff-target is up to date.
Changes applied before test
commit 996f34060329868877ec238e3630f949f2da0c83
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri May 13 11:08:24 2022 +0200

    BaseLoader: Fix handling of exceptions derived directly from BaseException
    
    This reverts f510a00d10b3451161d3af756e415b9916d6d8fa to replace it
    with a fix of the underlying issue.

See https://jenkins.softwareheritage.org/job/DLDBASE/job/tests-on-diff/789/ for more details.

unrevert change to the position of 'success = False'

(you probably want to update the commit message because you're not reverting the other commit anymore !)

This revision is now accepted and ready to land.Fri, May 13, 11:32 AM
This revision was landed with ongoing or failed builds.Fri, May 13, 11:34 AM
This revision was automatically updated to reflect the committed changes.

Build is green

Patch application report for D7828 (id=28269)

Rebasing onto f510a00d10...

Current branch diff-target is up to date.
Changes applied before test
commit dc32b112457de28d536affa65d54b99146bea679
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri May 13 11:08:24 2022 +0200

    BaseLoader: Fix handling of exceptions derived directly from BaseException
    
    This reverts f510a00d10b3451161d3af756e415b9916d6d8fa to replace it
    with a fix of the underlying issue.

See https://jenkins.softwareheritage.org/job/DLDBASE/job/tests-on-diff/790/ for more details.

It may be worth adding a test for a loader getting a BaseException derivate in one of the early methods too. But that's a bit nitpicky.

Build is green

Patch application report for D7828 (id=28270)

Rebasing onto f510a00d10...

Current branch diff-target is up to date.
Changes applied before test
commit 3874cdfe4647396c49559f05524dd01b8795c699
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri May 13 11:08:24 2022 +0200

    BaseLoader: Fix handling of exceptions derived directly from BaseException

See https://jenkins.softwareheritage.org/job/DLDBASE/job/tests-on-diff/791/ for more details.