joshelser commented on a change in pull request #37:
URL: https://github.com/apache/phoenix-queryserver/pull/37#discussion_r438513805
##########
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:
As a meta-comment, I don't know why I felt the need to distinguish the
'avatica_user' from the normal 'user'. I think I had some idea where PQS could
provide some authz and then the database has its own? I wonder if this is just
too complicated.
----------------------------------------------------------------
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]
