Merge ~philroche/simplestreams/+git/simplestreams:feature/restore-older-python-requests-support into simplestreams:master

Proposed by Philip Roche
Status: Merged
Approved by: Philip Roche
Approved revision: fb7e4fd3741521160c79588610d89d417489b6a8
Merge reported by: Server Team CI bot
Merged at revision: not available
Proposed branch: ~philroche/simplestreams/+git/simplestreams:feature/restore-older-python-requests-support
Merge into: simplestreams:master
Diff against target: 28 lines (+8/-2)
1 file modified
simplestreams/contentsource.py (+8/-2)
Reviewer Review Type Date Requested Status
Paride Legovini Approve
Server Team CI bot continuous-integration Approve
Adam Collard (community) Approve
Review via email: mp+412585@code.qastaging.launchpad.net

Commit message

Restores support for older versions of python-requests

If simplestreams is being used with a version of
python-requests < 2.4.1 then any calls to request.get
will result in
```
ValueError: Timeout value connect was (TIMEOUT, None), but it must
be an int or float.
```
The option to use a tuple of (content timeout,read timeout) was
in troduced in requests 2.4.1. Making this chance restores support
for systems with python-requests < 2.4.1 while still retaining the
support for the timeout feature which now will continue to behave
like the non requests Urllib2UrlReader class.

To post a comment you must log in.
Revision history for this message
Philip Roche (philroche) wrote :

You can see the change in requests timeout api from float to tuple|float @ https://docs.python-requests.org/en/v2.3.0/api/ and https://docs.python-requests.org/en/v2.4.3/api/

Revision history for this message
Paride Legovini (paride) wrote :

Hi Philip, thanks for this MP. May I ask you in which environment is simplestreams used with such an old version of requests? According to rmadison we have to go back to Trusty, as Xenial already had 2.9.1.

review: Needs Information
Revision history for this message
Philip Roche (philroche) wrote :

@paride The env in question has python-requests with 2.2.1.

Revision history for this message
Adam Collard (adam-collard) :
review: Needs Fixing
Revision history for this message
Philip Roche (philroche) wrote :

@paride @adam-collard Addressed your concerns

@adam-collard RE separate class - I moved the format change to after the `if requests is None` check which I think addresses your concern.

Revision history for this message
Adam Collard (adam-collard) wrote :

LGTM

review: Approve
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Paride Legovini (paride) wrote :

CI is happy. Now to make this land change the MP status from "Needs review" to "Approved" (that's the main MP status, not a review comment). CI will detect this and squash-merge the MP.

Revision history for this message
Philip Roche (philroche) wrote :

@Paride. Will do - are you +1 now?

Revision history for this message
Paride Legovini (paride) wrote :

Yep!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches