Page MenuHomeSoftware Heritage
Differential D6630

Clean up loader, add types and update docstrings
ClosedPublic
Actions

Authored by ardumont on Nov 9 2021, 6:20 PM.

Details

Reviewers
vlorentz
Group Reviewers
Reviewers
Maniphest Tasks
T3719: loader svn fails to clean up properly disk when it crashes
Commits
rDLDSVN9ddf7115b8e6: Clean up loader, add types and update docstrings
Summary

This also drops either no longer used or ineffective parameters from the loader to ease
readability.

Related to T3719

Diff Detail

Repository
rDLDSVN Subversion (SVN) loader
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ardumont created this revision.Nov 9 2021, 6:20 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptNov 9 2021, 6:20 PM
swh-public-ci added a comment.Nov 9 2021, 6:21 PM

Build is green

Patch application report for D6630 (id=24094)

Rebasing onto e36caff0a4...

Current branch diff-target is up to date.
Changes applied before test
commit 0ba8accc6c8770684468d1498f4eb81e015bbd4b
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Nov 9 18:18:00 2021 +0100

    Clean up loader, add types and more docstring
    
    This drops either no longer used or ineffective parameters from the loader.
    
    Related to T3719

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

Harbormaster completed remote builds in B24969: Diff 24094.Nov 9 2021, 6:21 PM
ardumont requested review of this revision.Nov 9 2021, 6:21 PM
ardumont updated this revision to Diff 24095.Nov 9 2021, 7:01 PM

Update more docstrings

ardumont retitled this revision from Clean up loader, add types and more docstring to Clean up loader, add types and update docstrings.Nov 9 2021, 7:01 PM
ardumont edited the summary of this revision. (Show Details)
ardumont added a subscriber: anlambert.
ardumont added inline comments.
swh/loader/svn/loader.py
388

Now that the post_load actually execute that check as the last step, i'm wondering whether we want that check to stay at all.

@anlambert btw ^ (just worth mentioning, if we decide to drop it, it will be in another diff)

swh-public-ci added a comment.Nov 9 2021, 7:03 PM

Build is green

Patch application report for D6630 (id=24095)

Rebasing onto e36caff0a4...

Current branch diff-target is up to date.
Changes applied before test
commit 15718d3205d95c58f5d2785c29bcd65c003dfcba
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Nov 9 18:18:00 2021 +0100

    Clean up loader, add types and update docstrings
    
    This also drops either no longer used or ineffective parameters from the loader to ease
    readability.
    
    Related to T3719

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

Harbormaster completed remote builds in B24970: Diff 24095.Nov 9 2021, 7:03 PM
ardumont updated this revision to Diff 24096.Nov 9 2021, 7:06 PM

Align docstring definition with task module

swh-public-ci added a comment.Nov 9 2021, 7:08 PM

Build is green

Patch application report for D6630 (id=24096)

Rebasing onto e36caff0a4...

Current branch diff-target is up to date.
Changes applied before test
commit a3d88d518bf8e961c6309b6d279189321d85fc57
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Nov 9 18:18:00 2021 +0100

    Clean up loader, add types and update docstrings
    
    This also drops either no longer used or ineffective parameters from the loader to ease
    readability.
    
    Related to T3719

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

Harbormaster completed remote builds in B24971: Diff 24096.Nov 9 2021, 7:08 PM
ardumont updated this revision to Diff 24099.Nov 10 2021, 10:09 AM

Add more types

swh-public-ci added a comment.Nov 10 2021, 10:11 AM

Build is green

Patch application report for D6630 (id=24099)

Rebasing onto e36caff0a4...

Current branch diff-target is up to date.
Changes applied before test
commit 21d0d9cb99f4a5c25d7cd45f7fde54ea4436e293
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Nov 9 18:18:00 2021 +0100

    Clean up loader, add types and update docstrings
    
    This also drops either no longer used or ineffective parameters from the loader to ease
    readability.
    
    Related to T3719

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

Harbormaster completed remote builds in B24974: Diff 24099.Nov 10 2021, 10:11 AM
ardumont updated this revision to Diff 24101.Nov 10 2021, 10:33 AM

last update

swh-public-ci added a comment.Nov 10 2021, 10:35 AM

Build is green

Patch application report for D6630 (id=24101)

Rebasing onto e36caff0a4...

Current branch diff-target is up to date.
Changes applied before test
commit 9ddf7115b8e6bc3825bc5e931a23608226450fc6
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Nov 9 18:18:00 2021 +0100

    Clean up loader, add types and update docstrings
    
    This also drops either no longer used or ineffective parameters from the loader to ease
    readability.
    
    Related to T3719

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

Harbormaster completed remote builds in B24976: Diff 24101.Nov 10 2021, 10:35 AM
vlorentz accepted this revision.Nov 10 2021, 11:17 AM
vlorentz added a subscriber: vlorentz.

Why is check_revision not a boolean?

This revision is now accepted and ready to land.Nov 10 2021, 11:17 AM
ardumont added a comment.Nov 10 2021, 11:20 AM

Why is check_revision not a boolean?

iirc, to allow the user to tell how many revisions in between check they want to do.
If it's a boolean, you need to hardcode it to a pre-defined number.

And that check you don't really want to do it at each revision [1] as an svn export instruction is involved.

[1] for debug purposes it's fine but not for production.

swh/loader/svn/loader.py
388

@vlorentz regarding your previous question, here is the check if you want to follow through from there ;)

Closed by commit rDLDSVN9ddf7115b8e6: Clean up loader, add types and update docstrings (authored by ardumont). · Explain WhyNov 10 2021, 11:29 AM
This revision was automatically updated to reflect the committed changes.
ardumont added a commit: rDLDSVN9ddf7115b8e6: Clean up loader, add types and update docstrings.
vlorentz mentioned this in T3137: Add type annotations to swh.loaders.svn.Apr 12 2022, 10:28 AM

Revision Contents
Changeset List

PathSize
swh/
loader/
svn/
146 lines
14 lines
7 lines
tests/
54 lines

Diff 24105

View Options

swh/loader/svn/loader.py

Loading...
View Options

swh/loader/svn/svn.py

Loading...
View Options

swh/loader/svn/tasks.py

Loading...
View Options

swh/loader/svn/tests/test_loader.py

Loading...
About · Developer information · Legal