Skip to content
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
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

blueyed
Copy link

@blueyed blueyed commented Sep 12, 2017

@blueyed
Copy link
Author

blueyed commented Nov 22, 2017

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

@ruxkor
Copy link

ruxkor commented Dec 12, 2017

@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
Copy link
Author

blueyed commented Jan 9, 2018

@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
Copy link
Author

blueyed commented Jan 9, 2018

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.

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants