Paths

Table of Contentst

Differential D5077

Rework loader instantiation logic according to loader core api
ClosedPublic
Actions

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

Details

Reviewers

anlambert

Group Reviewers

Reviewers

Maniphest Tasks

T1410: Kill implicit configuration: new configuration scheme

Commits

rDLDGb14c06e70846: 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.

impacts:

D5093: swh.web
D5092: swh.vault

Related to T1410

Test Plan

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

Diff Detail

Repository

rDLDG Git loader

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ardumont created this revision.Feb 15 2021, 6:02 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptFeb 15 2021, 6:02 PM

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!

ardumont mentioned this in D5071: Unify loader instantiation.Feb 15 2021, 6:12 PM

Adapt according to D5088

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.

Harbormaster completed remote builds in B19285: Diff 18184.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, 1:46 PM

ardumont edited the summary of this revision. (Show Details)Feb 17 2021, 1:49 PM

ardumont edited the summary of this revision. (Show Details)Feb 17 2021, 2:15 PM

ardumont mentioned this in D5094: api_route: Ensure never_cache is honored for all response status codes.Feb 17 2021, 2:25 PM

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

ardumont added inline comments.Feb 17 2021, 2:55 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.

Harbormaster completed remote builds in B19290: Diff 18189.Feb 17 2021, 3:06 PM

ardumont marked 3 inline comments as done.Feb 17 2021, 3:06 PM

Closed by commit rDLDGb14c06e70846: Rework loader instantiation logic according to loader core api (authored by ardumont). · Explain WhyFeb 17 2021, 3:06 PM

This revision was automatically updated to reflect the committed changes.

ardumont added a commit: rDLDGb14c06e70846: Rework loader instantiation logic according to loader core api.