cjbj commented on PR #48734: URL: https://github.com/apache/airflow/pull/48734#issuecomment-2819678830
@guan404ming @Lee-W @h30306 This (merged) PR raised some questions. If `get_uri()` is for "[a alid SQLAlchemy URI](https://github.com/apache/airflow/pull/48734#issue-2968972335)" then the [change in the MR](https://github.com/apache/airflow/blob/providers-oracle/4.0.3/providers/oracle/src/airflow/providers/oracle/hooks/oracle.py#L461-L467): ``` uri = f"oracle://{login}:{password}@{host}:{port}" if sid: uri = f"{uri}/{sid}" elif service_name: uri = f"{uri}/{service_name}" elif conn.schema: uri = f"{uri}/{conn.schema}" ``` may not be correct since it doesn't follow [documented SQLAlchemy connection syntax for the python-oracledb driver](https://docs.sqlalchemy.org/en/20/dialects/oracle.html#dialect-oracle-oracledb-connect). - Since airflow 3 now uses python-oracledb, the URI should begin with `oracle+oracledb:` otherwise the obsolete cx_Oracle driver would be needed. - SQLAlchemy needs service names to be prefixed with `?service_name=`. - Service names are the new way forward (as of a few decades ago), so `service_name` should be first choice, not `sid` in the if/else block. - I forget the history of Airflow and schemas, so can't comment on whether the final `elif` block needs some tweaks to be valid SQLAlchemy syntax. Code should be like: ``` uri = f"oracle+oracledb://{login}:{password}@{host}:{port}" if service_name: uri = f"{uri}?service_name={service_name}" elif sid: uri = f"{uri}/{sid}" # and maybe this, depending what is in conn.schema???? #elif conn.schema: # uri = f"{uri}/{conn.schema}" ``` -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
