Page MenuHomeSoftware Heritage

Rework loader instantiation logic according to loader core api
ClosedPublic

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

Details

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
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 19296
Build 29924: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 29923: arc lint + arc unit

Event Timeline

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!

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!

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.

anlambert added a subscriber: anlambert.

Looks good to me !

requirements.txt
11

Looks like that requirement is not needed.

swh/loader/mercurial/tasks.py
16–18

You should add typing to parameters here.

34–36

same here

swh/loader/mercurial/tasks_from_disk.py
16–18

same here

33–35

same here

This revision is now accepted and ready to land.Feb 17 2021, 3:17 PM

Adapt according to review (add type annotations on tasks)

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.

There is still the iso8601 dependency to remove.

  • Drop the unneeded iso8601 requirement.
  • Add missing tests on parse_visit_date

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 ¯\_(ツ)_/¯

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.