Page MenuHomeSoftware Heritage

svn: Capture svnrdump failure as not_found instead of failed
ClosedPublic

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

Details

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

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.

swh/loader/svn/loader.py
687–690

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.

752–753

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.

swh/loader/svn/loader.py
687–690

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 ;)

Drop the unneeded changes, thanks for the heads up!

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.

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)

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 ;)

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.

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.

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
53

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

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

We can declare list of error codes and messages here:

error_codes = []
error_messages = []
689–690

Replace with:

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

Raise NotFound here:

if "E170013" in error_codes:
   ....
751–754

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

Adapt to a better implementation

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.

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

swh/loader/svn/loader.py
691

s/lines/line/

This revision now requires changes to proceed.Sep 29 2021, 7:07 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.

This revision is now accepted and ready to land.Sep 29 2021, 7:16 PM