Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support older versions of urllib3 and Databricks Runtime with regards to DEFAULT_METHOD_WHITELIST change to DEFAULT_ALLOWED_METHODS #240

Merged
merged 3 commits into from
Jul 18, 2023

Conversation

mgyucht
Copy link
Contributor

@mgyucht mgyucht commented Jul 18, 2023

Changes

This PR changes the SDK to tolerate urllib3 < 1.26.0. In these versions of urllib3, Retry class's DEFAULT_ALLOWED_METHODS was called DEFAULT_METHOD_WHITELIST, and the parameter in Retry's constructor was changed from method_whitelist to allowed_methods. With this change, users using older versions of DBR (like the 7.3 LTS) can still use the current version of the SDK, even though the version of urllib3 is old.

This change can be reverted when we no longer need to support urllib3 < 1.26.0 (i.e. after DBR 10.4 LTS, which is EOL on March 18, 2025).

Test

  • Manually tested on DBR 7.3LTS: Old_DBR_test_-_Databricks
  • Manually tested on DBR 13.1:
    Old_DBR_test_-_Databricks_2

@mgyucht mgyucht requested review from nfx and pietern July 18, 2023 07:39
@codecov-commenter
Copy link

codecov-commenter commented Jul 18, 2023

Codecov Report

Patch coverage: 66.66% and no project coverage change.

Comparison is base (6f93548) 53.24% compared to head (68d0229) 53.24%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #240   +/-   ##
=======================================
  Coverage   53.24%   53.24%           
=======================================
  Files          32       32           
  Lines       19151    19154    +3     
=======================================
+ Hits        10197    10199    +2     
- Misses       8954     8955    +1     
Impacted Files Coverage Δ
databricks/sdk/core.py 67.38% <66.66%> (-0.01%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@mgyucht mgyucht changed the title Support old versions of urllib3 Support old versions of urllib3/DBR Jul 18, 2023
Copy link
Contributor

@nfx nfx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@nfx nfx changed the title Support old versions of urllib3/DBR Support older versions of urllib3 and Databricks Runtime with regards to DEFAULT_METHOD_WHITELIST change to DEFAULT_ALLOWED_METHODS Jul 18, 2023
@nfx nfx merged commit 25ff035 into main Jul 18, 2023
7 checks passed
@nfx nfx deleted the support-old-urllib3 branch July 18, 2023 14:01
nfx added a commit that referenced this pull request Jul 18, 2023
* Support older versions of `urllib3` and Databricks Runtime with regards to `DEFAULT_METHOD_WHITELIST` change to `DEFAULT_ALLOWED_METHODS` ([#240](#240)).
@nfx nfx mentioned this pull request Jul 18, 2023
nfx added a commit that referenced this pull request Jul 18, 2023
* Support older versions of `urllib3` and Databricks Runtime with
regards to `DEFAULT_METHOD_WHITELIST` change to
`DEFAULT_ALLOWED_METHODS`
([#240](#240)).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants