Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings
This repository was archived by the owner on Mar 18, 2019. It is now read-only.

Fix handling of deprecated credentials in HTTPTransport#146

Open
blueyed wants to merge 3 commits into
core-api:mastercore-api/python-client:masterfrom
blueyed:fix-deprecated-credentialsblueyed/core-api-python-client:fix-deprecated-credentialsCopy head branch name to clipboard
Open

Fix handling of deprecated credentials in HTTPTransport#146
blueyed wants to merge 3 commits into
core-api:mastercore-api/python-client:masterfrom
blueyed:fix-deprecated-credentialsblueyed/core-api-python-client:fix-deprecated-credentialsCopy head branch name to clipboard

Conversation

@blueyed

@blueyed blueyed commented Sep 12, 2017

Copy link
Copy Markdown

@blueyed

blueyed commented Nov 22, 2017

Copy link
Copy Markdown
Author

Ping @tomchristie.
This is an important fix for coreapi-cli: core-api/coreapi-cli#19.

@ruxkor

ruxkor commented Dec 12, 2017

Copy link
Copy Markdown

@blueyed Why the stacklevel=2 ?

I'd simply solve the issue by parsing credentials first and only then auth. This potential overwrites session.auth, in accordance to the (already existing) warning.

diff --git a/coreapi/transports/http.py b/coreapi/transports/http.py
index a548024..2ee3337 100644
--- a/coreapi/transports/http.py
+++ b/coreapi/transports/http.py
@@ -339,17 +339,17 @@ class HTTPTransport(BaseTransport):
             headers = {key.lower(): value for key, value in headers.items()}
         if session is None:
             session = requests.Session()
-        if auth is not None:
-            session.auth = auth
-        if not getattr(session.auth, 'allow_cookies', False):
-            session.cookies.set_policy(BlockAll())
-
         if credentials is not None:
             warnings.warn(
                 "The 'credentials' argument is now deprecated in favor of 'auth'.",
                 DeprecationWarning
             )
-            auth = DomainCredentials(credentials)
+            session.auth = DomainCredentials(credentials)
+        if auth is not None:
+            session.auth = auth
+        if not getattr(session.auth, 'allow_cookies', False):
+            session.cookies.set_policy(BlockAll())
+
         if request_callback is not None or response_callback is not None:
             warnings.warn(
                 "The 'request_callback' and 'response_callback' arguments are now deprecated. "

@blueyed

blueyed commented Jan 9, 2018

Copy link
Copy Markdown
Author

@ruxkor
I am not sure anymore (this is rather old already).
Please consider creating a separate PR from your patch linking to this one.

It does not seem to be well-maintained currently though, but if somebody gets to merge fixes it would be good to have your alternative ready.

@blueyed

blueyed commented Jan 9, 2018

Copy link
Copy Markdown
Author

Why the stacklevel=2 ?

This is necessary to get the location of where the code is actually used, i.e. where it needs to be fixed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.