Build correct SQLAlchemy URI in Teradata Hook #56305
Conversation
guan404ming
left a comment
guan404ming
left a comment
There was a problem hiding this comment.
Functional wise looks nice, but I'm not that familiar with Teradata. Need another eyes to check on this. Thanks!
| except ImportError: | ||
| from airflow.models.connection import Connection # type: ignore[assignment] | ||
|
|
||
| DEFAULT_DB_PORT = 1025 |
There was a problem hiding this comment.
nit: Probably could add a link to the doc that mentions 1025 as a comment above this line.
|
|
||
| return conn_config | ||
|
|
||
| def get_sqlalchemy_engine(self, engine_kwargs=None): |
There was a problem hiding this comment.
This is technically a breaking change. @eladkal do we need to do anything?
There was a problem hiding this comment.
@sc250072 can you raise PR to re add the function to preserve backward compatibility and raise deprecation warning for it?
There was a problem hiding this comment.
It will fall back to this method and return the same URL from sqlalchemy_url. But any case,
I’ll re-add the function with a deprecation warning to preserve backward compatibility.
There was a problem hiding this comment.
This is technically a breaking change. @eladkal do we need to do anything?
Could you please clarify what you mean by a breaking change?
Are you referring to the latest Teradata provider not being compatible with older Airflow versions?
There was a problem hiding this comment.
one who uses older Teradata provider might break. (as this method has been removed and might be used), but if we're to bump major version. that should be fine
| @property | ||
| def sqlalchemy_url(self) -> URL: | ||
| """ | ||
| Override to return a Sqlalchemy.engine.URL object from the Teradata connection. |
There was a problem hiding this comment.
| Override to return a Sqlalchemy.engine.URL object from the Teradata connection. | |
| Override to return a `sqlalchemy.engine.URL` object from the Teradata connection. |
There was a problem hiding this comment.
I'm a bit confused. override what to return the object?
There was a problem hiding this comment.
4 participants
Related: #38195
Why
The default implementation of DbApiHook.get_uri does not conform to the standard Teradata connection format.
How
Override sqlalchemy_url property to follow the official Teradata SQLAlchemy URI format documented here:
https://pypi.org/project/teradatasqlalchemy/#ConnectionParameters
Return the properly rendered SQLAlchemy URL in get_uri.