Page MenuHomeSoftware Heritage
Differential D5078

Rework loader instantiation logic according to loader core api
ClosedPublic
Actions

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

Details

Reviewers
anlambert
Group Reviewers
Reviewers
Maniphest Tasks
T1410: Kill implicit configuration: new configuration scheme
Commits
rDLDHGd8f28b70fa77: 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 types along the way.

Related to T1410

Test Plan

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

Diff Detail

Repository
rDLDHG Mercurial loader
Branch
improve-instantiation
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 19226
Build 29818: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 29817: arc lint + arc unit

Event Timeline

ardumont created this revision.Feb 15 2021, 6:12 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptFeb 15 2021, 6:12 PM
ardumont mentioned this in D5071: Unify loader instantiation.Feb 15 2021, 6:12 PM
swh-public-ci added a comment.Feb 15 2021, 6:12 PM

Build has FAILED

Patch application report for D5078 (id=18122)

Rebasing onto 71cfd10018...

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

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

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

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 15 2021, 6:12 PM
Harbormaster failed remote builds in B19226: Diff 18122!
ardumont updated this revision to Diff 18181.Feb 17 2021, 1:07 PM

Adapt according to D5088

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

Build has FAILED

Patch application report for D5078 (id=18181)

Rebasing onto 71cfd10018...

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

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

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

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 17 2021, 1:07 PM
Harbormaster failed remote builds in B19282: Diff 18181!
swh-public-ci added a comment.Feb 17 2021, 1:26 PM

Build is green

Patch application report for D5078 (id=18181)

Rebasing onto 71cfd10018...

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

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

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

Harbormaster completed remote builds in B19282: Diff 18181.Feb 17 2021, 1:26 PM
ardumont requested review of this revision.Feb 17 2021, 1:26 PM
ardumont edited the summary of this revision. (Show Details)Feb 17 2021, 2:16 PM
anlambert accepted this revision.Feb 17 2021, 3:17 PM
anlambert added a subscriber: anlambert.

Looks good to me !

requirements.txt
11

Looks like that requirement is not needed.

swh/loader/mercurial/tasks.py
14

You should add typing to parameters here.

30

same here

swh/loader/mercurial/tasks_from_disk.py
14

same here

29

same here

This revision is now accepted and ready to land.Feb 17 2021, 3:17 PM
ardumont updated this revision to Diff 18195.Feb 17 2021, 3:36 PM

Adapt according to review (add type annotations on tasks)

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

Build is green

Patch application report for D5078 (id=18195)

Rebasing onto 71cfd10018...

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

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

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

Harbormaster completed remote builds in B19296: Diff 18195.Feb 17 2021, 3:38 PM
anlambert added a comment.Feb 17 2021, 3:59 PM

There is still the iso8601 dependency to remove.

ardumont updated this revision to Diff 18197.Feb 17 2021, 4:06 PM
  • Drop the unneeded iso8601 requirement.
  • Add missing tests on parse_visit_date
ardumont added a comment.Feb 17 2021, 4:06 PM

There is still the iso8601 dependency to remove.

yep, i was working on adding the missing test on parse_visit_date (since i moved it).

requirements.txt
11

ah yeah, it's not needed, i'll remove it, thanks.

I started this refacto (like svn loader one) and thought i'd need it.
But in the end, there is the parsing which still uses the dateutil stuff.
That will end up being removed in another diff.

swh/loader/mercurial/utils.py
29

should raise here ¯\_(ツ)_/¯

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

Build is green

Patch application report for D5078 (id=18197)

Rebasing onto 71cfd10018...

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

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

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

Harbormaster completed remote builds in B19298: Diff 18197.Feb 17 2021, 4:07 PM
Closed by commit rDLDHGd8f28b70fa77: Rework loader instantiation logic according to loader core api (authored by ardumont). · Explain WhyFeb 17 2021, 4:08 PM
This revision was automatically updated to reflect the committed changes.
ardumont added a commit: rDLDHGd8f28b70fa77: Rework loader instantiation logic according to loader core api.

Revision Contents
Changeset List

PathSize
2 lines
1 line
swh/
loader/
mercurial/
98 lines
84 lines
17 lines
12 lines
tests/
48 lines
25 lines
74 lines
29 lines

Diff 18122

View Options

requirements-swh.txt

Loading...
View Options

requirements.txt

Loading...
View Options

swh/loader/mercurial/from_disk.py

Loading...
View Options

swh/loader/mercurial/loader.py

Loading...
View Options

swh/loader/mercurial/tasks.py

Loading...
View Options

swh/loader/mercurial/tasks_from_disk.py

Loading...
View Options

swh/loader/mercurial/tests/conftest.py

Loading...
View Options

swh/loader/mercurial/tests/test_from_disk.py

Loading...
View Options

swh/loader/mercurial/tests/test_loader.py

Loading...
View Options

swh/loader/mercurial/utils.py

Loading...
About · Developer information · Legal