-
Notifications
You must be signed in to change notification settings - Fork 112
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Issue339 ldap logmech #347
Changes from 5 commits
908d1b6
2c292f3
05705cf
c9f2db0
74512a5
11f8880
9459987
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,7 +38,7 @@ def verify(expr, params=None): | |
|
||
|
||
def connect( | ||
host: str, user_name: str, password: str, port: Optional[int] = 1025 | ||
host: str, user_name: str, password: str, port: Optional[int] = 1025, logmech: Optional[str]='TD2' | ||
) -> TeradataClient: | ||
""" Create a TeradataClient for use with Ibis. | ||
Parameters | ||
|
@@ -49,11 +49,13 @@ def connect( | |
A Database username to connect with | ||
password : str | ||
Password for supplied username | ||
logmech : Optional[str] | ||
Logmech flag to select with (default. TD2) | ||
port : Optional[int] | ||
The database port to connect to (default. 1025) | ||
Returns | ||
------- | ||
TeradataClient | ||
""" | ||
|
||
return TeradataClient(host, user_name, password, port) | ||
return TeradataClient(host, user_name, password, logmech, port) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This won't work since the TeradataClient init method takes the parameters in a different order. I would put logmech after port and make it Optional. The teradatasql.connect() documentation shows that "TD2" should be the default logmech There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated in the new commit. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Logmech is still before port. When instantiating a TeradataClient class, this will fail. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -66,7 +66,7 @@ class TeradataClient(SQLClient): | |
table_class = TeradataTable | ||
dialect = compiler.TeradataDialect | ||
|
||
def __init__(self, host, user_name, password, port=1025, use_no_lock_tables=False): | ||
def __init__(self, host, user_name, password, port=1025, logmech="TD2", use_no_lock_tables=False): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if default arguments can be written in double quotes. Pretty sure it has to be |
||
"""Construct a TeradataClient. | ||
|
||
Parameters | ||
nehanene15 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
@@ -85,6 +85,7 @@ def __init__(self, host, user_name, password, port=1025, use_no_lock_tables=Fals | |
"user": user_name, | ||
"password": password, | ||
"dbs_port": port, | ||
"logmech": logmech, | ||
} | ||
|
||
self.client = teradatasql.connect(**self.teradata_config) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Can we update this to reflect the order of the parameters? (logmech after port)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep!