Page MenuHomeSoftware Heritage

check_snapshot: Check existence down to contents
ClosedPublic

Authored by ardumont on Jul 10 2020, 6:14 PM.

Details

Summary

This now checks from the top to bottom that all objects referenced by a
snapshot exist.

I'm opening the diff as a correctness first approach. Did not refactor the
common code yet.

Also i believe i'm missing the nested directory check scenario (but that's not really a blocker).

Related to T2483
Depends on D3502

Test Plan

tox

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

Build is green

Patch application report for D3503 (id=12384)

Could not rebase; Attempt merge onto 9f04d72516...

Updating 9f04d72..6e0c592
Fast-forward
 swh/loader/tests/__init__.py  | 63 ++++++++++++++++++++++++++++++++----
 swh/loader/tests/test_init.py | 74 +++++++++++++++++++++++++++++++++++++++----
 2 files changed, 123 insertions(+), 14 deletions(-)
Changes applied before test
commit 6e0c592fcb477ba8543542efdeb4f6c941623bbf
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Jul 10 18:13:16 2020 +0200

    check_snapshot: Check existence down to the contents
    
    Related to T2484

commit e45e9fc1ef369390ccf49e7110f68b259779fb8e
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Jul 10 17:39:54 2020 +0200

    check_snapshot: Check existence up to the first level directories
    
    targeted by revisions
    
    Related to T2484

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

Build is green

Patch application report for D3503 (id=12386)

Could not rebase; Attempt merge onto 9f04d72516...

Updating 9f04d72..940f60f
Fast-forward
 swh/loader/tests/__init__.py  | 63 +++++++++++++++++++++++++++++++++-----
 swh/loader/tests/test_init.py | 70 ++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 119 insertions(+), 14 deletions(-)
Changes applied before test
commit 940f60fcaa347b77d7a0f09e93db231f9f8b4851
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Jul 10 18:13:16 2020 +0200

    check_snapshot: Check existence down to the contents
    
    Related to T2484

commit 54cc0adc5b1e42b3cd9951781530b62f6515648d
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Jul 10 17:39:54 2020 +0200

    check_snapshot: Check existence up to the first level directories
    
    targeted by revisions
    
    Related to T2484

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

douardda added a subscriber: douardda.

looks ok, but I find it temptating to factorize(!) the missing objects looking pattern (not asked right now!).

Also, in the commit message, is it "Check existence down to the contents" or "Check existence down to contents"?

This revision is now accepted and ready to land.Jul 15 2020, 12:12 PM

looks ok, but I find it temptating to factorize(!) the missing objects looking pattern (not asked right now!).

Yes, agreed (i mentioned it in the description)
I wanted to check with you if the approach is sound.

And then refactor, but then week-end...
yeah, agreed, we can do that later.

Also, in the commit message, is it "Check existence down to the contents" or "Check existence down to contents"?

"down to contents" sounds better.
I'll update.

ardumont retitled this revision from check_snapshot: Check existence down to the contents to check_snapshot: Check existence down to contents.Jul 15 2020, 12:25 PM
ardumont edited the summary of this revision. (Show Details)

Build is green

Patch application report for D3503 (id=12401)

Rebasing onto 54cc0adc5b...

Current branch diff-target is up to date.
Changes applied before test
commit e1034bfe2744777dad465a772e005762b8fb1fdc
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Jul 10 18:13:16 2020 +0200

    check_snapshot: Check existence down to contents
    
    Prior to this commit, that checked from existence from top (snapshot) up to
    directories. This goes one extra step and iterate over directories and contents
    to check they exist as well.
    
    Related to T2484

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