Page MenuHomeSoftware Heritage

Rework loader instantiation logic according to loader core api
ClosedPublic

Authored by ardumont on Feb 15 2021, 6:02 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 type along the way.

impacts:

Related to T1410

Test Plan

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

Diff Detail

Repository
rDLDG Git loader
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 19290
Build 29916: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 29915: arc lint + arc unit

Event Timeline

Build has FAILED

Patch application report for D5077 (id=18118)

Rebasing onto c2cd5fed02...

Current branch diff-target is up to date.
Changes applied before test
commit e35f0e44367b2be52447679ed17eeb197babbea6
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sun Feb 7 09:18:14 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/DLDG/job/tests-on-diff/82/
See console output for more information: https://jenkins.softwareheritage.org/job/DLDG/job/tests-on-diff/82/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 B19222: Diff 18118!

Build has FAILED

Patch application report for D5077 (id=18184)

Rebasing onto c2cd5fed02...

Current branch diff-target is up to date.
Changes applied before test
commit f0191192e81be429969305cd94fd989f6d63097f
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sun Feb 7 09:18:14 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/DLDG/job/tests-on-diff/83/
See console output for more information: https://jenkins.softwareheritage.org/job/DLDG/job/tests-on-diff/83/console

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 17 2021, 1:10 PM
Harbormaster failed remote builds in B19285: Diff 18184!

Build is green

Patch application report for D5077 (id=18184)

Rebasing onto c2cd5fed02...

Current branch diff-target is up to date.
Changes applied before test
commit f0191192e81be429969305cd94fd989f6d63097f
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sun Feb 7 09:18:14 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/DLDG/job/tests-on-diff/84/ for more details.

anlambert added a subscriber: anlambert.

Looks good to me !

swh/loader/git/from_disk.py
48

maybe logging_class could be set to the real loader class name ?

swh/loader/git/loader.py
126

same here

163

why the type ignore here ?

This revision is now accepted and ready to land.Feb 17 2021, 2:49 PM
swh/loader/git/from_disk.py
48

yes, i could even drop it now as the default is to fallback to what you suggest (in loader-core).

swh/loader/git/loader.py
163

(because it complained ;)

I don't recall the details but it was not happy, probably because of the optional nature on stuff.

Adapt according to review:

  • drop logging_class
  • Fix the type ignore

Build is green

Patch application report for D5077 (id=18189)

Rebasing onto c2cd5fed02...

Current branch diff-target is up to date.
Changes applied before test
commit b14c06e7084665cd239866f71c0a2604537f9712
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sun Feb 7 09:18:14 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/DLDG/job/tests-on-diff/85/ for more details.