[GitHub] [phoenix-queryserver] stoty commented on a change in pull request #60: PHOENIX-6459 Try to use a Session to pass along any cookies the serve…

[GitBox <gi...@apache.org>]

stoty commented on a change in pull request #60:
URL: https://github.com/apache/phoenix-queryserver/pull/60#discussion_r624888623



##########
File path: python-phoenixdb/phoenixdb/avatica/client.py
##########
@@ -149,6 +149,7 @@ def __init__(self, url, max_retries=None, auth=None, verify=None):
         self.auth = auth
         self.verify = verify
         self.connection = None
+        self.session = requests.Session()

Review comment:
       IMO this would better in open_connection()
   

##########
File path: python-phoenixdb/phoenixdb/avatica/client.py
##########
@@ -173,7 +174,7 @@ def _post_request(self, body, headers):
                 requestArgs.update(verify=self.verify)

Review comment:
       Wouldn't performing the authentication setup  on the Session object in open_connection() once per sessions be better?
   
   We could also pre-set the content-type on the Session object, and wouldn't have to pass the headers parameter around.
   
   The same could be done for the stream flag.
   
   https://docs.python-requests.org/en/master/user/advanced/#session-objects
   https://2.python-requests.org/en/master/api/#request-sessions




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org