Fix typing issue
ClosedPublic
Actions

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

Details

Reviewers

anlambert
olasd

Group Reviewers

Reviewers

Commits

rDLDGd7481af6b33c: Fix typing issue

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

Branch

master

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 25888
Build 40460: Phabricator diff pipeline on jenkins	Jenkins console · Jenkins
Build 40459: arc lint + arc unit

Event Timeline

vlorentz created this revision.Jan 10 2022, 5:01 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptJan 10 2022, 5:01 PM

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.

vlorentz added a child revision: D6901: Remove unnecessary use of dulwich.client.HttpGitClient.Jan 10 2022, 5:47 PM

done in D6901

fix parentheses

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.

Harbormaster completed remote builds in B25893: Diff 25023.Jan 10 2022, 5:54 PM

vlorentz requested review of this revision.Jan 10 2022, 5:54 PM

anlambert accepted this revision.Jan 10 2022, 6:02 PM

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

olasd accepted this revision.Jan 10 2022, 6:35 PM

Closed by commit rDLDGd7481af6b33c: Fix typing issue (authored by vlorentz). · Explain WhyJan 10 2022, 6:41 PM

This revision was automatically updated to reflect the committed changes.

vlorentz added a commit: rDLDGd7481af6b33c: Fix typing issue.