Skip to content

Remove use of method_whitelist when possible#532

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

Merged
merged 1 commit into from
Feb 10, 2021
Merged

Remove use of method_whitelist when possible #532

merged 1 commit into from
Feb 10, 2021

Conversation

xmo-odoo
Copy link
Contributor

@xmo-odooxmo-odoo commented Feb 8, 2021

Deprecated in favor of allowed_methods. Fall back to the old argument for older
versions of urllib3 which do not support the new one.

Uses conditional attribute check as recommended in
urllib3/urllib3#2057

Fixes#515

RELEASE NOTE: Fixed a bug that prevented the proper operation of the SDK when used with certain versions of the urllib3 package.

@google-cla
Copy link

google-clabot commented Feb 8, 2021

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@xmo-odoo
Copy link
ContributorAuthor

@googlebot I signed it!

Copy link
Contributor

@hiranya911hiranya911 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @xmo-odoo. This looks great! I just made a couple of comments. We can merge this once those comments are addressed.

@xmo-odoo
Copy link
ContributorAuthor

xmo-odoo commented Feb 9, 2021

Thanks for the PR @xmo-odoo. This looks great! I just made a couple of comments. We can merge this once those comments are addressed.

Done and done, though now that I look at the code again I'm wondering: maybe I should remove the current _ANY_METHOD constant and rename method_filter to that? It would provide essentially the same information (that you want to retry on every HTTP method by default) without an essentially unnecessary extra constant. **_ANY_METHOD might read slightly odd, but not that much.

That is, end up with this:

ifhasattr(retry.Retry.DEFAULT, 'allowed_methods'): _ANY_METHOD= {'allowed_methods': None} else: _ANY_METHOD= {'method_whitelist': None} # Default retry configuration: Retries once on low-level connection and socket read errors.# Retries up to 4 times on HTTP 500 and 503 errors, with exponential backoff. Returns the# last response upon exhausting all retries.DEFAULT_RETRY_CONFIG=retry.Retry( connect=1, read=1, status=4, status_forcelist=[500, 503], raise_on_status=False, backoff_factor=0.5, **_ANY_METHOD)
@hiranya911
Copy link
Contributor

Thanks @xmo-odoo. Your suggestion looks good to me. Feel free to make the change. But I'm happy with the current state of the PR too.

Deprecated in favor of allowed_methods. Fall back to the old argument for older versions of urllib3 which do not support the new one. Uses conditional attribute check as recommended in urllib3/urllib3#2057
@xmo-odoo
Copy link
ContributorAuthor

@hiranya911 I pushed the updated version.

@hiranya911hiranya911 merged commit 8c05981 into firebase:masterFeb 10, 2021
@xmo-odooxmo-odoo deleted the urllib3-whitelist-deprecation branch February 11, 2021 07:48
sts-odoo added a commit to odoo-dev/odoo that referenced this pull request Sep 28, 2022
With version of urllib3 in ubuntu 22.04 (1.26.5) and with version of firebase_admin between 2.17.0 and 4.5.2 a gives the following warning: In firebase_admin/_http_client.py:30: DeprecationWarning: Using 'method_whitelist' with Retry is deprecated and will be removed in v2.0. Use 'allowed_methods' instead It has been fixed in later version firebase/firebase-admin-python#532 since v4.5.2
robodoo pushed a commit to odoo/odoo that referenced this pull request Sep 30, 2022
With version of urllib3 in ubuntu 22.04 (1.26.5) and with version of firebase_admin between 2.17.0 and 4.5.2 a gives the following warning: In firebase_admin/_http_client.py:30: DeprecationWarning: Using 'method_whitelist' with Retry is deprecated and will be removed in v2.0. Use 'allowed_methods' instead It has been fixed in later version firebase/firebase-admin-python#532 since v4.5.2 closes#101444 Signed-off-by: Christophe Monniez (moc) <moc@odoo.com>
robodoo pushed a commit to odoo/odoo that referenced this pull request Sep 30, 2022
With version of urllib3 in ubuntu 22.04 (1.26.5) and with version of firebase_admin between 2.17.0 and 4.5.2 a gives the following warning: In firebase_admin/_http_client.py:30: DeprecationWarning: Using 'method_whitelist' with Retry is deprecated and will be removed in v2.0. Use 'allowed_methods' instead It has been fixed in later version firebase/firebase-admin-python#532 since v4.5.2 closes#101444 Signed-off-by: Christophe Monniez (moc) <moc@odoo.com>
fw-bot pushed a commit to odoo-dev/odoo that referenced this pull request Sep 30, 2022
With version of urllib3 in ubuntu 22.04 (1.26.5) and with version of firebase_admin between 2.17.0 and 4.5.2 a gives the following warning: In firebase_admin/_http_client.py:30: DeprecationWarning: Using 'method_whitelist' with Retry is deprecated and will be removed in v2.0. Use 'allowed_methods' instead It has been fixed in later version firebase/firebase-admin-python#532 since v4.5.2 X-original-commit: 66934cd
robodoo pushed a commit to odoo/odoo that referenced this pull request Oct 6, 2022
With version of urllib3 in ubuntu 22.04 (1.26.5) and with version of firebase_admin between 2.17.0 and 4.5.2 a gives the following warning: In firebase_admin/_http_client.py:30: DeprecationWarning: Using 'method_whitelist' with Retry is deprecated and will be removed in v2.0. Use 'allowed_methods' instead It has been fixed in later version firebase/firebase-admin-python#532 since v4.5.2 closes#101731 X-original-commit: 66934cd Signed-off-by: Christophe Monniez (moc) <moc@odoo.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 participants
@xmo-odoo@hiranya911
close