Make backwards compatible with urllib3~=1.0 [Follow up #197] #206
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
susodapop
requested review from
arikfr,
yunbodeng-db and
andrefurlan-db
as code owners
benc-db
approved these changes
Aug 23, 2023
|
The python 3.7 checks are failing because our poetry github action fails with no details. Investigating. |
added 3 commits
August 24, 2023 11:49
We already implement the equivalent of backoff_max so the behaviour will be the same for urllib3==1.x and urllib3==2.x We do not implement backoff jitter so the behaviour for urllib3==1.x will NOT include backoff jitter whereas urllib3==2.x WILL include jitter. Signed-off-by: Jesse Whitehouse <[email protected]>
Signed-off-by: Jesse Whitehouse <[email protected]>
fix from #208. Signed-off-by: Jesse Whitehouse <[email protected]>
|
Fixed the actions failure in #208. Rebased onto |
|
Checks all pass now 🚀 |
susodapop
pushed a commit
to unj1m/databricks-sql-python
that referenced
this pull request
Sep 19, 2023
databricks#206) * Make retry policy backwards compatible with urllib3~=1.0.0 We already implement the equivalent of backoff_max so the behaviour will be the same for urllib3==1.x and urllib3==2.x We do not implement backoff jitter so the behaviour for urllib3==1.x will NOT include backoff jitter whereas urllib3==2.x WILL include jitter. --------- Signed-off-by: Jesse Whitehouse <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Not sure how we missed this when testing #197, but
urllib3~=1.xdoes not implementbackoff_maxorbackoff_jitterconfigurations. So executions would fail for users who installedurllib31=1.x. This PR completes the work of #197 and updates our retry test mocks to be compatible with the internal workings of the older urllib3 version.urllib3 version 1 expects the response to include a
.msgattribute with.keys()and.items()methods. While in real-life execution this would be anHTTPMessageobject, thedict()that our mocks use for headers is already sufficient and implements those methods..msgis ignored in urllib3 version 2 so the same mock definition works regardless of which version of urllib3 is installed.Going forward, we should have some kind of smoke test that runs our tests with both versions of urllib3 installed. A bare install of this connector always pulls urllib3 version 2, but many customers and partners will have already installed an older urllib3 -- this case isn't captured in our current testing pattern.
Related Tickets & Documents
Resolves #205
Resolves ES-836977