Page MenuHomeSoftware Heritage
Differential D5075

Rework loader instantiation logic according to loader core api
ClosedPublic
Actions

Authored by ardumont on Feb 15 2021, 6:00 PM.

Details

Reviewers
anlambert
Group Reviewers
Reviewers
Maniphest Tasks
T1410: Kill implicit configuration: new configuration scheme
Commits
rDLDSVN2c54129bcf48: Rework loader instantiation logic according to loader core api
Summary

The new loader.core now passes all configuration through the constructor.
So no more default configuration only constructor parameters.

Note that this also updated some docstrings and type along the way.

Related to T1410

Test Plan

tox
(failing until swh.loader.core > 0.17 is released)

Diff Detail

Repository
rDLDSVN Subversion (SVN) loader
Branch
improve-instantiation
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 19283
Build 29903: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 29902: arc lint + arc unit

Event Timeline

ardumont created this revision.Feb 15 2021, 6:00 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptFeb 15 2021, 6:00 PM
ardumont added a child revision: D5076: loader: Expect visit_date as an optional date in constructors.Feb 15 2021, 6:00 PM
swh-public-ci added a comment.Feb 15 2021, 6:01 PM

Build has FAILED

Patch application report for D5075 (id=18116)

Rebasing onto 71e3b44784...

Current branch diff-target is up to date.
Changes applied before test
commit 727189d56849edd8481db1d6d4b7919c1e935aff
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sun Feb 7 08:45:16 2021 +0100

    Rework loader instantiation logic according to loader core api
    
    Note that this also updated some docstrings and type along the way.

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

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 15 2021, 6:01 PM
Harbormaster failed remote builds in B19220: Diff 18116!
ardumont edited the summary of this revision. (Show Details)Feb 15 2021, 6:01 PM
ardumont added a task: T1410: Kill implicit configuration: new configuration scheme.
ardumont updated this revision to Diff 18119.Feb 15 2021, 6:03 PM

Rework commit message

swh-public-ci added a comment.Feb 15 2021, 6:03 PM

Build has FAILED

Patch application report for D5075 (id=18119)

Rebasing onto 71e3b44784...

Current branch diff-target is up to date.
Changes applied before test
commit 2d6423cfd454afb07f3f309a9076f95300640936
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sun Feb 7 08:45:16 2021 +0100

    Rework loader instantiation logic according to loader core api
    
    Note that this also updated some docstrings and type along the way.
    
    Related to T1410

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

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 15 2021, 6:03 PM
Harbormaster failed remote builds in B19223: Diff 18119!
ardumont updated this revision to Diff 18120.Feb 15 2021, 6:04 PM

Update requirement-swh.txt

swh-public-ci added a comment.Feb 15 2021, 6:04 PM

Build has FAILED

Patch application report for D5075 (id=18120)

Rebasing onto 71e3b44784...

Current branch diff-target is up to date.
Changes applied before test
commit 69b218f8e22298d21d39b11893c3045bb6cedec8
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sun Feb 7 08:45:16 2021 +0100

    Rework loader instantiation logic according to loader core api
    
    Note that this also updated some docstrings and type along the way.
    
    Related to T1410

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

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 15 2021, 6:04 PM
Harbormaster failed remote builds in B19224: Diff 18120!
ardumont mentioned this in D5071: Unify loader instantiation.Feb 15 2021, 6:12 PM
ardumont updated this revision to Diff 18182.Feb 17 2021, 1:08 PM

Adapt according to D5088

swh-public-ci added a comment.Feb 17 2021, 1:08 PM

Build has FAILED

Patch application report for D5075 (id=18182)

Rebasing onto 71e3b44784...

Current branch diff-target is up to date.
Changes applied before test
commit 5ad2709b12881a8b8a65488a82d7dfc5a0cbb48c
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sun Feb 7 08:45:16 2021 +0100

    Rework loader instantiation logic according to loader core api
    
    Note that this also updated some docstrings and type along the way.
    
    Related to T1410

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

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 17 2021, 1:08 PM
Harbormaster failed remote builds in B19283: Diff 18182!
swh-public-ci added a comment.Feb 17 2021, 1:25 PM

Build is green

Patch application report for D5075 (id=18182)

Rebasing onto 71e3b44784...

Current branch diff-target is up to date.
Changes applied before test
commit 5ad2709b12881a8b8a65488a82d7dfc5a0cbb48c
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sun Feb 7 08:45:16 2021 +0100

    Rework loader instantiation logic according to loader core api
    
    Note that this also updated some docstrings and type along the way.
    
    Related to T1410

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

Harbormaster completed remote builds in B19283: Diff 18182.Feb 17 2021, 1:25 PM
ardumont requested review of this revision.Feb 17 2021, 1:25 PM
ardumont edited the summary of this revision. (Show Details)Feb 17 2021, 2:15 PM
anlambert accepted this revision.Feb 17 2021, 5:45 PM
anlambert added a subscriber: anlambert.

Looks good to me !

swh/loader/svn/loader.py
76

would be simpler to use an int with a default value of 0 here

The test if self.check_revision will still behave the same.

93

self.check_revision = check_revision if you remove Optional.

This revision is now accepted and ready to land.Feb 17 2021, 5:45 PM
ardumont added inline comments.Feb 17 2021, 5:54 PM
swh/loader/svn/loader.py
76

I should default to 1000 as before to avoid changing behavior.

Nonetheless, ack on your suggestion.

It's not clear why i used Optional here...
Maybe because i was in bot mode :D

ardumont updated this revision to Diff 18210.Feb 17 2021, 5:56 PM

Adapt according to review

swh-public-ci added a comment.Feb 17 2021, 5:57 PM

Build is green

Patch application report for D5075 (id=18210)

Rebasing onto 71e3b44784...

Current branch diff-target is up to date.
Changes applied before test
commit 7476a936423ff78f5c81af4262d3e82350deb6f0
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sun Feb 7 08:45:16 2021 +0100

    Rework loader instantiation logic according to loader core api
    
    Note that this also updated some docstrings and type along the way.
    
    Related to T1410

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

Harbormaster completed remote builds in B19308: Diff 18210.Feb 17 2021, 5:57 PM
anlambert added inline comments.Feb 17 2021, 5:59 PM
swh/loader/svn/loader.py
75

I think you should set it to 0 as default value as revision checks was not enabled by default previously.

ardumont added inline comments.Feb 17 2021, 5:59 PM
swh/loader/svn/loader.py
75

yes, agreed

76

apparently i misremembered, it's not checking by default ¯\_(ツ)_/¯... (also checked swh-site).

ardumont updated this revision to Diff 18212.Feb 17 2021, 6:16 PM

Set check_revision to 0

Harbormaster completed remote builds in B19310: Diff 18212.Feb 17 2021, 6:16 PM
ardumont updated this revision to Diff 18213.Feb 17 2021, 6:16 PM

Use correct repo to update the diff ¯\_(ツ)_/¯

swh-public-ci added a comment.Feb 17 2021, 6:17 PM

Build is green

Patch application report for D5075 (id=18213)

Rebasing onto 71e3b44784...

Current branch diff-target is up to date.
Changes applied before test
commit 2c54129bcf4864537d8d13d6645ef777fc1186ef
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sun Feb 7 08:45:16 2021 +0100

    Rework loader instantiation logic according to loader core api
    
    Note that this also updated some docstrings and type along the way.
    
    Related to T1410

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

Harbormaster completed remote builds in B19311: Diff 18213.Feb 17 2021, 6:17 PM
Closed by commit rDLDSVN2c54129bcf48: Rework loader instantiation logic according to loader core api (authored by ardumont). · Explain WhyFeb 17 2021, 6:19 PM
This revision was automatically updated to reflect the committed changes.
ardumont added a commit: rDLDSVN2c54129bcf48: Rework loader instantiation logic according to loader core api.

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
5 lines--
2 lines--
1 line--
swh/
loader/
svn/
129 lines83%
Loading...
76 lines100%
Loading...
tests/
44 lines100%
Loading...
92 lines100%
Loading...

Diff 18182

View Options

mypy.ini

Loading...
View Options

requirements-swh.txt

Loading...
View Options

requirements.txt

Loading...
View Options

swh/loader/svn/loader.py

Loading...
View Options

swh/loader/svn/tasks.py

Loading...
View Options

swh/loader/svn/tests/conftest.py

Loading...
View Options

swh/loader/svn/tests/test_loader.py

Loading...
About · Developer information · Legal