Page MenuHomeSoftware Heritage

Remove unnecessary use of dulwich.client.HttpGitClient
ClosedPublic

Authored by vlorentz on Jan 10 2022, 5:47 PM.

Details

Summary

'requests' does the job just fine with less complexity.

Depends on D6900.

Diff Detail

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

Event Timeline

Build is green

Patch application report for D6901 (id=25021)

Could not rebase; Attempt merge onto 0cc96c25ab...

Updating 0cc96c2..2e43e1c
Fast-forward
 swh/loader/git/dumb.py | 36 ++++++++++++------------------------
 1 file changed, 12 insertions(+), 24 deletions(-)
Changes applied before test
commit 2e43e1cafb5b0fdfdd6eb7601b13303cb9001a8d
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Jan 10 17:47:13 2022 +0100

    Remove unnecessary use of dulwich.client.HttpGitClient
    
    'requests' does the job just fine with less complexity.

commit c56f6126703aeaabd7b2a37e0960f39e5e7257d7
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Jan 10 17:01:34 2022 +0100

    Fix typing issue
    
    response.content_type is set by Dulwich, but isn't part of urllib3's
    HTTPResponse, so we shouldn't rely on it.
    (And it makes mypy complain when the 'types-urllib3' package is installed)

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

Build is green

Patch application report for D6901 (id=25024)

Could not rebase; Attempt merge onto 0cc96c25ab...

Updating 0cc96c2..76739ec
Fast-forward
 swh/loader/git/dumb.py | 36 ++++++++++++------------------------
 1 file changed, 12 insertions(+), 24 deletions(-)
Changes applied before test
commit 76739ece7af9313564dc5a1e7ffec10475da3e48
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Jan 10 17:47:13 2022 +0100

    Remove unnecessary use of dulwich.client.HttpGitClient
    
    'requests' does the job just fine with less complexity.

commit d7481af6b33c3b7ddaad31f1801074d1525a7af2
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Jan 10 17:01:34 2022 +0100

    Fix typing issue
    
    response.content_type is set by Dulwich, but isn't part of urllib3's
    HTTPResponse, so we shouldn't rely on it.
    (And it makes mypy complain when the 'types-urllib3' package is installed)

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

anlambert added a subscriber: anlambert.
anlambert added inline comments.
swh/loader/git/dumb.py
116

Use a requests.Session initialized in the class constructor instead as numerous requests can be sent with the dumb protocol.

This revision now requires changes to proceed.Jan 10 2022, 5:55 PM
swh/loader/git/dumb.py
116

oh, I assumed there was a default session, but it looks like there isn't

vlorentz marked an inline comment as done.

use a session

Build is green

Patch application report for D6901 (id=25026)

Could not rebase; Attempt merge onto 0cc96c25ab...

Updating 0cc96c2..705976f
Fast-forward
 swh/loader/git/dumb.py | 37 +++++++++++++------------------------
 1 file changed, 13 insertions(+), 24 deletions(-)
Changes applied before test
commit 705976f75bc30aa1451babe10b5449b91afce2bf
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Jan 10 17:47:13 2022 +0100

    Remove unnecessary use of dulwich.client.HttpGitClient
    
    'requests' does the job just fine with less complexity.

commit d7481af6b33c3b7ddaad31f1801074d1525a7af2
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Jan 10 17:01:34 2022 +0100

    Fix typing issue
    
    response.content_type is set by Dulwich, but isn't part of urllib3's
    HTTPResponse, so we shouldn't rely on it.
    (And it makes mypy complain when the 'types-urllib3' package is installed)

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

This revision is now accepted and ready to land.Jan 10 2022, 6:03 PM