Page MenuHomeSoftware Heritage

Rework loader instantiation logic according to loader core api
ClosedPublic

Authored by ardumont on Mon, Feb 15, 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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

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.Mon, Feb 15, 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.Wed, Feb 17, 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
12 ↗(On Diff #18181)

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.Wed, Feb 17, 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
12 ↗(On Diff #18181)

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.