Page MenuHomeSoftware Heritage

Fix typing issue
ClosedPublic

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

Details

Summary

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)

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

Build has FAILED

Patch application report for D6900 (id=25018)

Rebasing onto 0cc96c25ab...

Current branch diff-target is up to date.
Changes applied before test
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)

Link to build: https://jenkins.softwareheritage.org/job/DLDG/job/tests-on-diff/163/
See console output for more information: https://jenkins.softwareheritage.org/job/DLDG/job/tests-on-diff/163/console

Harbormaster returned this revision to the author for changes because remote builds failed.Jan 10 2022, 5:02 PM
Harbormaster failed remote builds in B25888: Diff 25018!

Reading the rest of this file, we're not using features from dulwich's HttpGitClient all that much, it would probably make sense to replace it altogether with a basic requests session?

In D6900#179442, @olasd wrote:

Reading the rest of this file, we're not using features from dulwich's HttpGitClient all that much, it would probably make sense to replace it altogether with a basic requests session?

For the record, I preferred to use the features available in dulwich when implementing this instead of adding a new dependency to requests in the loader.
But yes, this could be easily ported to requests.

Effectively requests is in our "standard library" (via swh.core[http]), so it's not a big deal.
Either way, we need parens around the or statement (and has higher precedence) though.

Build is green

Patch application report for D6900 (id=25023)

Rebasing onto 0cc96c25ab...

Current branch diff-target is up to date.
Changes applied before test
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/166/ for more details.

This revision is now accepted and ready to land.Jan 10 2022, 6:02 PM
This revision was automatically updated to reflect the committed changes.