Page MenuHomeSoftware Heritage
Differential D6373

svn: Capture svnrdump failure as not_found instead of failed
ClosedPublic
Actions

Authored by ardumont on Sep 29 2021, 2:30 PM.

Details

Reviewers
anlambert
Group Reviewers
Reviewers
Maniphest Tasks
T3613: svn: An error occurred when running svnrdump and no exploitable dump file has been generated.
Commits
rDLDSVN666f32a01cc3: svn: Capture svnrdump failure as not_found instead of failed
Summary

Currently, those are failing with a 'failed' status. This commit fixes such status to
the correct 'not_found' one.

Related to T3613

Test Plan

tox

Diff Detail

Event Timeline

ardumont created this revision.Sep 29 2021, 2:30 PM
swh-public-ci added a comment.Sep 29 2021, 2:33 PM

Build is green

Patch application report for D6373 (id=23171)

Rebasing onto b01318f80a...

Current branch diff-target is up to date.
Changes applied before test
commit 89fffffef56e19c3c820cdbe39b657066fceac46
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Sep 29 14:28:17 2021 +0200

    svn: Stop considering not found repositories as failure
    
    This captures svnrdump failures due to no longer existing repositories as not_found.
    This updates the status accordingly in the archive.
    
    Related to T3613

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

Harbormaster completed remote builds in B24087: Diff 23171.Sep 29 2021, 2:33 PM
ardumont requested review of this revision.Sep 29 2021, 2:33 PM
anlambert added inline comments.Sep 29 2021, 2:48 PM
swh/loader/svn/loader.py
687–692

How about capturing the error codes with a regexp here instead ?
This would allow to handle other ones in the future if we found some new errors.

754–755

Instead of processing stderr lines again, I would rather capture error codes in a list and check its content.

swh/loader/svn/tests/test_loader.py
98–99

To remove I guess.

149

Not sure if that variable should have been renamed in all other tests.

ardumont added inline comments.Sep 29 2021, 4:02 PM
swh/loader/svn/loader.py
687–692

do you have an example in mind for both that suggestion and the next?

I'm a bit hazy on how to do it.

swh/loader/svn/tests/test_loader.py
98–99

yes, it's done locally, i forgot to update the diff.

149

oh no, right ;)

ardumont updated this revision to Diff 23192.Sep 29 2021, 4:04 PM

Drop the unneeded changes, thanks for the heads up!

swh-public-ci added a comment.Sep 29 2021, 4:05 PM

Build is green

Patch application report for D6373 (id=23192)

Rebasing onto b01318f80a...

Current branch diff-target is up to date.
Changes applied before test
commit a74b1f49ab465ed5dbdc761749d6ae733babf126
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Sep 29 14:28:17 2021 +0200

    svn: Stop considering not found repositories as failure
    
    This captures svnrdump failures due to no longer existing repositories as not_found.
    This updates the status accordingly in the archive.
    
    Related to T3613

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

Harbormaster completed remote builds in B24109: Diff 23192.Sep 29 2021, 4:06 PM
ardumont retitled this revision from svn: Stop considering not found repositories as failure to svn: Capture svnrdump not failures to mark them as not_found.Sep 29 2021, 6:30 PM
ardumont edited the summary of this revision. (Show Details)
ardumont retitled this revision from svn: Capture svnrdump not failures to mark them as not_found to svn: Capture svnrdump failure as not_found instead of failed.
ardumont edited the summary of this revision. (Show Details)
ardumont updated this revision to Diff 23195.Sep 29 2021, 6:32 PM

Adapt according to suggestion.

@anlambert i did not use a list, we'll see when we have another failing issues to
capture ;)

Otherwise, i adapted according to my understanding of your suggestion.

As usual, thanks ;)

swh-public-ci added a comment.Sep 29 2021, 6:34 PM

Build is green

Patch application report for D6373 (id=23195)

Rebasing onto b01318f80a...

Current branch diff-target is up to date.
Changes applied before test
commit a6198d10c106d1c2d9b707dfd284250576147e3d
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Sep 29 14:28:17 2021 +0200

    svn: Capture svnrdump failure as not_found instead of failed
    
    Currently, those are failing with a 'failed' status. This commit fixes such status to
    the correct 'not_found' one.
    
    Related to T3613

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

Harbormaster completed remote builds in B24112: Diff 23195.Sep 29 2021, 6:34 PM
ardumont updated this revision to Diff 23197.Sep 29 2021, 6:42 PM

Update test docstring

ardumont marked 3 inline comments as done.Sep 29 2021, 6:42 PM
swh-public-ci added a comment.Sep 29 2021, 6:44 PM

Build is green

Patch application report for D6373 (id=23197)

Rebasing onto b01318f80a...

Current branch diff-target is up to date.
Changes applied before test
commit 57566d3480002ac07178817604359d13ece0e456
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Sep 29 14:28:17 2021 +0200

    svn: Capture svnrdump failure as not_found instead of failed
    
    Currently, those are failing with a 'failed' status. This commit fixes such status to
    the correct 'not_found' one.
    
    Related to T3613

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

Harbormaster completed remote builds in B24114: Diff 23197.Sep 29 2021, 6:44 PM
anlambert requested changes to this revision.Sep 29 2021, 6:50 PM

We should capture all svn errors in order to:

  • raise exception based on an error code
  • give more details about the failure by appending error messages to fallback exception
swh/loader/svn/loader.py
52

I was rather thinking of capturing all error codes and messages, with the following regex.

SUBVERSION_ERROR = re.compile(r".*(E[0-9]{6}):.*")
668

We can declare list of error codes and messages here:

error_codes = []
error_messages = []
688–689

Replace with:

match = SUBVERSION_ERROR.search(line)
if match:
    error_codes.append(match.group(1)
    error_messages.append(lines)
752–757

Raise NotFound here:

if "E170013" in error_codes:
   ....
758–761

Put error messages in exception text:

raise Exception(
    "An error occurred when running svnrdump and "
     "no exploitable dump file has been generated.\n" +
     "\n".join(error_messages)
)
This revision now requires changes to proceed.Sep 29 2021, 6:50 PM
ardumont updated this revision to Diff 23198.Sep 29 2021, 7:00 PM

Adapt to a better implementation

swh-public-ci added a comment.Sep 29 2021, 7:02 PM

Build is green

Patch application report for D6373 (id=23198)

Rebasing onto b01318f80a...

Current branch diff-target is up to date.
Changes applied before test
commit fb720931e61347805af117c108685d962b449440
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Sep 29 14:28:17 2021 +0200

    svn: Capture svnrdump failure as not_found instead of failed
    
    Currently, those are failing with a 'failed' status. This commit fixes such status to
    the correct 'not_found' one.
    
    Related to T3613

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

Harbormaster completed remote builds in B24115: Diff 23198.Sep 29 2021, 7:02 PM
anlambert requested changes to this revision.Sep 29 2021, 7:07 PM

Wrong variable used in code (see inline comment), I will accept the diff afterwards.

swh/loader/svn/loader.py
692

s/lines/line/

This revision now requires changes to proceed.Sep 29 2021, 7:07 PM
ardumont updated this revision to Diff 23199.Sep 29 2021, 7:13 PM

Fix typo

swh-public-ci added a comment.Sep 29 2021, 7:15 PM

Build is green

Patch application report for D6373 (id=23199)

Rebasing onto b01318f80a...

Current branch diff-target is up to date.
Changes applied before test
commit 666f32a01cc3be0ce7f4591ad79ac6f7975b743e
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Sep 29 14:28:17 2021 +0200

    svn: Capture svnrdump failure as not_found instead of failed
    
    Currently, those are failing with a 'failed' status. This commit fixes such status to
    the correct 'not_found' one.
    
    Related to T3613

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

Harbormaster completed remote builds in B24116: Diff 23199.Sep 29 2021, 7:15 PM
anlambert accepted this revision.Sep 29 2021, 7:16 PM

Looks good to me !

This revision is now accepted and ready to land.Sep 29 2021, 7:16 PM
ardumont marked 5 inline comments as done.Sep 29 2021, 7:18 PM
Closed by commit rDLDSVN666f32a01cc3: svn: Capture svnrdump failure as not_found instead of failed (authored by ardumont). · Explain WhySep 29 2021, 7:19 PM
This revision was automatically updated to reflect the committed changes.
ardumont added a commit: rDLDSVN666f32a01cc3: svn: Capture svnrdump failure as not_found instead of failed.

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
swh/
loader/
svn/
20 lines87%
Loading...
tests/
21 lines100%
Loading...

Diff 23198

View Options

swh/loader/svn/loader.py

Loading...
View Options

swh/loader/svn/tests/test_loader.py

Loading...
About · Developer information · Legal