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



##########
File path: python/phoenixdb/phoenixdb/__init__.py
##########
@@ -101,33 +111,65 @@ def connect(url, max_retries=None, auth=None, 
authentication=None, avatica_user=
         :class:`~phoenixdb.connection.Connection` object.
     """
 
+    (url, auth, verify) = _process_args(
+        url, auth=auth, authentication=authentication,
+        avatica_user=avatica_user, avatica_password=avatica_password,
+        truststore=truststore, verify=verify, do_as=do_as, user=user, 
password=password)
+
+    client = AvaticaClient(url, max_retries=max_retries, auth=auth, 
verify=verify)
+    client.connect()
+    return Connection(client, **kwargs)
+
+
+def _process_args(
+        url, auth=None, authentication=None, avatica_user=None, 
avatica_password=None,
+        truststore=None, verify=None, do_as=None, user=None, password=None):
     url_parsed = urlparse(url)
     url_params = parse_qs(url_parsed.query, keep_blank_values=True)
 
-    # Parse supported JDBC compatible options from URL. args have precendece
-    rebuild = False
+    # Parse supported JDBC compatible parameters from URL. args have precendece
+    # Unlike the JDBC driver, we are expecting these as query params, as the 
avatica java client
+    # has a different idea of what an URL param is than urlparse. (urlparse 
seems just broken
+    # in this regard)
+    params_changed = False
     if auth is None and authentication is None and 'authentication' in 
url_params:
         authentication = url_params['authentication'][0]
         del url_params['authentication']
-        rebuild = True
+        params_changed = True
 
     if avatica_user is None and 'avatica_user' in url_params:
         avatica_user = url_params['avatica_user'][0]
         del url_params['avatica_user']
-        rebuild = True
+        params_changed = True
 
     if avatica_password is None and 'avatica_password' in url_params:
         avatica_password = url_params['avatica_password'][0]
         del url_params['avatica_password']
-        rebuild = True
+        params_changed = True
 
     if verify is None and truststore is None and 'truststore' in url_params:
         truststore = url_params['truststore'][0]
         del url_params['truststore']
-        rebuild = True
-
-    if rebuild:
-        url_parsed._replace(query=urlencode(url_params, True))
+        params_changed = True
+
+    if authentication == 'BASIC' or authentication == 'DIGEST':
+        # Handle standard user and password parameters
+        if user is not None and avatica_user is None:

Review comment:
       That would make sense, as Avatica is a kind of proxy. Its users don't 
seem to need that though.
   We could add similar alias options to the JDBC driver, maybe.




----------------------------------------------------------------
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:
[email protected]


Reply via email to