Page MenuHomeSoftware Heritage
Differential D7691

Store the result of MetadataFetcher.get_parent_origins
ClosedPublic
Actions

Authored by vlorentz on Apr 26 2022, 5:42 PM.

Details

Reviewers
olasd
Group Reviewers
Reviewers
Maniphest Tasks
T3273: Use "fork" relationships to speed-up initial load of large repositories
Commits
rDLDBASE07fb382655a0: Store the result of MetadataFetcher.get_parent_origins
Summary

So it is available to concrete loaders.

The Git loader will use it to incrementally load new origins by fetching
the last snapshot of a parent, instead of starting from scratch.

Diff Detail

Event Timeline

vlorentz created this revision.Apr 26 2022, 5:42 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptApr 26 2022, 5:42 PM
vlorentz added a parent revision: D7663: Add method get_parent_origins().Apr 26 2022, 5:42 PM
swh-public-ci added a comment.Apr 26 2022, 5:45 PM

Build is green

Patch application report for D7691 (id=27790)

Rebasing onto 1089330c3b...

Current branch diff-target is up to date.
Changes applied before test
commit 199dcb8f0683c6547b0e7bbb42e755a374fe6f7f
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Apr 26 17:41:09 2022 +0200

    Store the result of MetadataFetcher.get_parent_origins
    
    So it is available to concrete loaders.
    
    The Git loader will use it to incrementally load new origins by fetching
    the last snapshot of a parent, instead of starting from scratch.

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

Harbormaster completed remote builds in B28852: Diff 27790.Apr 26 2022, 5:45 PM
vlorentz requested review of this revision.Apr 26 2022, 5:45 PM
olasd added a subscriber: olasd.Apr 27 2022, 1:35 PM
olasd added inline comments.
swh/loader/core/metadata_fetchers.py
35

Should probably be Optional[List[Origin]]...

swh/loader/core/tests/test_loader.py
28

Please keep these consistent (I guess by always using the kwargs form)?

106–107

...because this one doesn't match the MetadataFetcherProtocol. Maybe it would be worth adding a test that all known/registered MetadataFetchers do match the protocol?

olasd accepted this revision.Apr 27 2022, 1:35 PM

LGTM (save from a couple of nitpicks), thanks

This revision is now accepted and ready to land.Apr 27 2022, 1:35 PM
vlorentz added inline comments.Apr 27 2022, 1:38 PM
swh/loader/core/tests/test_loader.py
106–107

I'll just updated this one instead.

And it's tested in swh-metadata-loaders. Or do you mean runtime checks?

vlorentz updated this revision to Diff 27819.Apr 27 2022, 1:40 PM
vlorentz marked 2 inline comments as done.

fix inconsistencies in tests

swh-public-ci added a comment.Apr 27 2022, 1:43 PM

Build has FAILED

Patch application report for D7691 (id=27819)

Rebasing onto 1089330c3b...

Current branch diff-target is up to date.
Changes applied before test
commit e05ea3a4d2c395564b61674ca52d2da79c090295
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Apr 26 17:41:09 2022 +0200

    Store the result of MetadataFetcher.get_parent_origins
    
    So it is available to concrete loaders.
    
    The Git loader will use it to incrementally load new origins by fetching
    the last snapshot of a parent, instead of starting from scratch.

Link to build: https://jenkins.softwareheritage.org/job/DLDBASE/job/tests-on-diff/745/
See console output for more information: https://jenkins.softwareheritage.org/job/DLDBASE/job/tests-on-diff/745/console

Harbormaster failed remote builds in B28880: Diff 27819!Apr 27 2022, 1:43 PM
olasd added inline comments.Apr 27 2022, 1:44 PM
swh/loader/core/tests/test_loader.py
106–107

No, I meant consistency checks at testing time. It's probably worth adding an ad-hoc test here for these two mock classes?

vlorentz updated this revision to Diff 27821.Apr 27 2022, 1:45 PM

fix test

vlorentz added inline comments.Apr 27 2022, 1:46 PM
swh/loader/core/tests/test_loader.py
106–107

I can, but it wouldn't have caught this issue; isinstance doesn't check type signatures. https://docs.python.org/3/library/typing.html#typing.runtime_checkable

swh-public-ci added a comment.Apr 27 2022, 1:48 PM

Build has FAILED

Patch application report for D7691 (id=27821)

Rebasing onto 1089330c3b...

Current branch diff-target is up to date.
Changes applied before test
commit 97999bf67068743279d1c9927556e8cb7383a3ba
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Apr 26 17:41:09 2022 +0200

    Store the result of MetadataFetcher.get_parent_origins
    
    So it is available to concrete loaders.
    
    The Git loader will use it to incrementally load new origins by fetching
    the last snapshot of a parent, instead of starting from scratch.

Link to build: https://jenkins.softwareheritage.org/job/DLDBASE/job/tests-on-diff/746/
See console output for more information: https://jenkins.softwareheritage.org/job/DLDBASE/job/tests-on-diff/746/console

Harbormaster failed remote builds in B28882: Diff 27821!Apr 27 2022, 1:48 PM
vlorentz updated this revision to Diff 27822.Apr 27 2022, 1:50 PM

fix test again + add isinstance check in tests

swh/loader/core/tests/test_loader.py
106–107

done

olasd added inline comments.Apr 27 2022, 1:51 PM
swh/loader/core/tests/test_loader.py
106–107

I mean, we do have such a set of explicit tests for our StorageInterface, ... etc. implementations.

I agree that this may be overkill for such a small interface, though. :-)

swh-public-ci added a comment.Apr 27 2022, 1:53 PM

Build is green

Patch application report for D7691 (id=27822)

Rebasing onto 1089330c3b...

Current branch diff-target is up to date.
Changes applied before test
commit 07fb382655a043f3e144fb35784d79635c564af9
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Apr 26 17:41:09 2022 +0200

    Store the result of MetadataFetcher.get_parent_origins
    
    So it is available to concrete loaders.
    
    The Git loader will use it to incrementally load new origins by fetching
    the last snapshot of a parent, instead of starting from scratch.

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

Harbormaster completed remote builds in B28883: Diff 27822.Apr 27 2022, 1:53 PM
vlorentz added inline comments.Apr 27 2022, 2:01 PM
swh/loader/core/tests/test_loader.py
106–107

oh, you're right. I'll consider it in swh-loader-metadata; but I don't think it's worth it to check test classes this deeply

vlorentz mentioned this in D7695: Replace 'base_url' argument with 'self.parent_origins' attribute.Apr 27 2022, 2:05 PM
vlorentz added a child revision: D7695: Replace 'base_url' argument with 'self.parent_origins' attribute.Apr 27 2022, 2:05 PM
vlorentz added a task: T3273: Use "fork" relationships to speed-up initial load of large repositories.Apr 27 2022, 2:07 PM
Closed by commit rDLDBASE07fb382655a0: Store the result of MetadataFetcher.get_parent_origins (authored by vlorentz). · Explain WhyApr 27 2022, 4:09 PM
This revision was automatically updated to reflect the committed changes.
vlorentz added a commit: rDLDBASE07fb382655a0: Store the result of MetadataFetcher.get_parent_origins.

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
swh/
loader/
core/
9 lines83%
Loading...
3 lines69%
Loading...
tests/
54 lines97%
Loading...

Diff 27822

View Options

swh/loader/core/loader.py

Loading...
View Options

swh/loader/core/metadata_fetchers.py

Loading...
View Options

swh/loader/core/tests/test_loader.py

Loading...
About · Developer information · Legal