You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/12/15 07:57:24 UTC
[GitHub] [airflow] mehmax opened a new pull request #19084: [Oracle] Oracle Hook - automatically set current_schema when defined in Connection
mehmax opened a new pull request #19084:
URL: https://github.com/apache/airflow/pull/19084
Per Default, Oracle Hook was using "scheme" field of the Connection only when generating DSN uri.
This PR adds functionality to Oracle Hook.
-> When a schema is provided in the Connection Parameters (maintained in the UI), then this schema is set as current_schema after connecting to the Database
This allows SQLs to be less static.
Before:
`SELECT * FROM PROD_SCHEMA.MY_TABLE`
`SELECT * FROM TEST_SCHEMA.MY_TABLE`
Now:
`SELECT * FROM MY_TABLE`
closes: #18664
**^ Add meaningful description above**
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] malthe commented on pull request #19084: [Oracle] Oracle Hook - automatically set current_schema when defined in Connection
Posted by GitBox <gi...@apache.org>.
malthe commented on pull request #19084:
URL: https://github.com/apache/airflow/pull/19084#issuecomment-1039208603
@mehmax I had a `conn.schema` which was an empty string so I needed the #21524 behavior there as well. I can confirm that rc3 works for me.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] boring-cyborg[bot] commented on pull request #19084: [Oracle] Oracle Hook - automatically set current_schema when defined in Connection
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on pull request #19084:
URL: https://github.com/apache/airflow/pull/19084#issuecomment-1031897507
Awesome work, congrats on your first merged pull request!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #19084: [Oracle] Oracle Hook - automatically set current_schema when defined in Connection
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #19084:
URL: https://github.com/apache/airflow/pull/19084#discussion_r769349906
##########
File path: airflow/providers/oracle/hooks/oracle.py
##########
@@ -90,8 +91,19 @@ def get_conn(self) -> 'OracleHook':
dsn = conn.host
if conn.port is not None:
dsn += ":" + str(conn.port)
- if service_name or conn.schema:
- dsn += "/" + (service_name or conn.schema)
+ if service_name:
+ dsn += "/" + service_name
+ elif conn.schema:
+ self.log.warning(
+ """You are using the parameter conn.schema to pass the Service Name
+ of your Oracle Database. In this version of the Hook, conn.schema
+ is additionally used to set the 'current_schema' attribute of the
+ Oracle DB Session.
+ Please use the correct parameter conn.extra.service_name instead to
+ prevent future issues.
+ More Info: https://github.com/apache/airflow/pull/19084"""
+ )
Review comment:
Can we make this a `DeprecationWarning`? There are multiple instances you can search as examples.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mehmax commented on a change in pull request #19084: [Oracle] Oracle Hook - automatically set current_schema when defined in Connection
Posted by GitBox <gi...@apache.org>.
mehmax commented on a change in pull request #19084:
URL: https://github.com/apache/airflow/pull/19084#discussion_r732812707
##########
File path: airflow/providers/oracle/hooks/oracle.py
##########
@@ -90,8 +91,8 @@ def get_conn(self) -> 'OracleHook':
dsn = conn.host
if conn.port is not None:
dsn += ":" + str(conn.port)
- if service_name or conn.schema:
- dsn += "/" + (service_name or conn.schema)
+ if service_name:
+ dsn += "/" + service_name
Review comment:
I removed it from the dsn because when people are using `schema` now, it could lead to a wrong dsn
eg. (`host:port/schema`).
Correct is only `host:port/service_name`
Do you think I should revert this change and the adapted tests?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr edited a comment on pull request #19084: [Oracle] Oracle Hook - automatically set current_schema when defined in Connection
Posted by GitBox <gi...@apache.org>.
uranusjr edited a comment on pull request #19084:
URL: https://github.com/apache/airflow/pull/19084#issuecomment-994466614
Sorry for missing this. This is close to ready IMO so let’s get it finished.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #19084: [Oracle] Oracle Hook - automatically set current_schema when defined in Connection
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #19084:
URL: https://github.com/apache/airflow/pull/19084#discussion_r733625839
##########
File path: airflow/providers/oracle/hooks/oracle.py
##########
@@ -90,8 +91,8 @@ def get_conn(self) -> 'OracleHook':
dsn = conn.host
if conn.port is not None:
dsn += ":" + str(conn.port)
- if service_name or conn.schema:
- dsn += "/" + (service_name or conn.schema)
+ if service_name:
+ dsn += "/" + service_name
Review comment:
No, it's OK, but this does mean we're making a backward incompatible change and will need to bump the provider's major version. I don't remember if you need to do anything specific in code to call this out, but can you edit the PR's description to make it clear this will need a new major version bump? (So the release manager can notice it when compiling the release.)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mehmax commented on a change in pull request #19084: [Oracle] Oracle Hook - automatically set current_schema when defined in Connection
Posted by GitBox <gi...@apache.org>.
mehmax commented on a change in pull request #19084:
URL: https://github.com/apache/airflow/pull/19084#discussion_r789159115
##########
File path: airflow/providers/oracle/hooks/oracle.py
##########
@@ -90,8 +91,19 @@ def get_conn(self) -> 'OracleHook':
dsn = conn.host
if conn.port is not None:
dsn += ":" + str(conn.port)
- if service_name or conn.schema:
- dsn += "/" + (service_name or conn.schema)
+ if service_name:
+ dsn += "/" + service_name
+ elif conn.schema:
+ self.log.warning(
+ """You are using the parameter conn.schema to pass the Service Name
+ of your Oracle Database. In this version of the Hook, conn.schema
+ is additionally used to set the 'current_schema' attribute of the
+ Oracle DB Session.
+ Please use the correct parameter conn.extra.service_name instead to
+ prevent future issues.
+ More Info: https://github.com/apache/airflow/pull/19084"""
+ )
Review comment:
Hi @uranusjr, sorry it took so long.
I changed the warning to `DeprecationWarning` and simplified the message.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk merged pull request #19084: [Oracle] Oracle Hook - automatically set current_schema when defined in Connection
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #19084:
URL: https://github.com/apache/airflow/pull/19084
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mehmax commented on pull request #19084: [Oracle] Oracle Hook - automatically set current_schema when defined in Connection
Posted by GitBox <gi...@apache.org>.
mehmax commented on pull request #19084:
URL: https://github.com/apache/airflow/pull/19084#issuecomment-1031739474
> The linter is complaining.
Should be fixed now. Sorry
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mehmax removed a comment on pull request #19084: [Oracle] Oracle Hook - automatically set current_schema when defined in Connection
Posted by GitBox <gi...@apache.org>.
mehmax removed a comment on pull request #19084:
URL: https://github.com/apache/airflow/pull/19084#issuecomment-1031739018
>
Should be fixed now :)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] malthe commented on pull request #19084: [Oracle] Oracle Hook - automatically set current_schema when defined in Connection
Posted by GitBox <gi...@apache.org>.
malthe commented on pull request #19084:
URL: https://github.com/apache/airflow/pull/19084#issuecomment-1038930988
This seems to have been fixed in https://github.com/apache/airflow/pull/21524 but I wonder if this was intentional in the sense that the `None` vs empty string situation was just resolved by chance.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mehmax commented on pull request #19084: [Oracle] Oracle Hook - automatically set current_schema when defined in Connection
Posted by GitBox <gi...@apache.org>.
mehmax commented on pull request #19084:
URL: https://github.com/apache/airflow/pull/19084#issuecomment-1031739018
>
Should be fixed now :)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mehmax commented on pull request #19084: [Oracle] Oracle Hook - automatically set current_schema when defined in Connection
Posted by GitBox <gi...@apache.org>.
mehmax commented on pull request #19084:
URL: https://github.com/apache/airflow/pull/19084#issuecomment-1039095741
Hi @malthe,
If you were passing the Service Name in the `conn.schema` field, the this PR might have had negative impact.
With fix in #21524 I not only changed this from a `None` to an empty string check, but also added `conn.extra["service_name"]` / `service_name` to the check
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mehmax commented on a change in pull request #19084: [Oracle] Oracle Hook - automatically set current_schema when defined in Connection
Posted by GitBox <gi...@apache.org>.
mehmax commented on a change in pull request #19084:
URL: https://github.com/apache/airflow/pull/19084#discussion_r732251123
##########
File path: airflow/providers/oracle/hooks/oracle.py
##########
@@ -90,8 +91,8 @@ def get_conn(self) -> 'OracleHook':
dsn = conn.host
if conn.port is not None:
dsn += ":" + str(conn.port)
- if service_name or conn.schema:
- dsn += "/" + (service_name or conn.schema)
+ if service_name:
+ dsn += "/" + service_name
Review comment:
With this PR I generally fixed the meaning of conn.schema.
In my opinion it was never correct to append conn.schema to the dsn.
According to [cx_Oracle Documentation](https://cx-oracle.readthedocs.io/en/latest/user_guide/connection_handling.html#easy-connect-syntax-for-connection-strings) easy connect dsn is built like this:
`dbhost.example.com:1984/orclpdb1`
where `orclpdb1` is the service_name, not the schema.
Without a service_name, cx_Oracle is not able to connect.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #19084: [Oracle] Oracle Hook - automatically set current_schema when defined in Connection
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #19084:
URL: https://github.com/apache/airflow/pull/19084#issuecomment-1031897290
Just a docker-compose failure which we will fix shorlty.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #19084: [Oracle] Oracle Hook - automatically set current_schema when defined in Connection
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #19084:
URL: https://github.com/apache/airflow/pull/19084#issuecomment-1039008208
> This seems to have been fixed in #21524 but I wonder if this was intentional in the sense that the `None` vs empty string situation was just resolved by chance.
This was the reason why I "held" the RC2 version for Oracle provider - and yeah, I believe @mehmax found the same compatibility provlem and fixed it (included in RC3). Any more comments @malthe about your error to verify it was the same issue?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] closed pull request #19084: [Oracle] Oracle Hook - automatically set current_schema when defined in Connection
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #19084:
URL: https://github.com/apache/airflow/pull/19084
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mehmax commented on a change in pull request #19084: [Oracle] Oracle Hook - automatically set current_schema when defined in Connection
Posted by GitBox <gi...@apache.org>.
mehmax commented on a change in pull request #19084:
URL: https://github.com/apache/airflow/pull/19084#discussion_r735822465
##########
File path: airflow/providers/oracle/hooks/oracle.py
##########
@@ -90,8 +91,8 @@ def get_conn(self) -> 'OracleHook':
dsn = conn.host
if conn.port is not None:
dsn += ":" + str(conn.port)
- if service_name or conn.schema:
- dsn += "/" + (service_name or conn.schema)
+ if service_name:
+ dsn += "/" + service_name
Review comment:
Hi @uranusjr, yes you are right. I thought of another solution now to make it more of a softer change.
Instead of not considering schema anymore, I added a warning to the user that they should switch to using conn.extra.service_name instead. WDYT?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #19084: [Oracle] Oracle Hook - automatically set current_schema when defined in Connection
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #19084:
URL: https://github.com/apache/airflow/pull/19084#discussion_r779289392
##########
File path: airflow/providers/oracle/hooks/oracle.py
##########
@@ -90,8 +91,19 @@ def get_conn(self) -> 'OracleHook':
dsn = conn.host
if conn.port is not None:
dsn += ":" + str(conn.port)
- if service_name or conn.schema:
- dsn += "/" + (service_name or conn.schema)
+ if service_name:
+ dsn += "/" + service_name
+ elif conn.schema:
+ self.log.warning(
+ """You are using the parameter conn.schema to pass the Service Name
+ of your Oracle Database. In this version of the Hook, conn.schema
+ is additionally used to set the 'current_schema' attribute of the
+ Oracle DB Session.
+ Please use the correct parameter conn.extra.service_name instead to
+ prevent future issues.
+ More Info: https://github.com/apache/airflow/pull/19084"""
+ )
Review comment:
Probably don’t need the More Info part; the first two sentences are good enough for me.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #19084: [Oracle] Oracle Hook - automatically set current_schema when defined in Connection
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #19084:
URL: https://github.com/apache/airflow/pull/19084#discussion_r732255335
##########
File path: airflow/providers/oracle/hooks/oracle.py
##########
@@ -90,8 +91,8 @@ def get_conn(self) -> 'OracleHook':
dsn = conn.host
if conn.port is not None:
dsn += ":" + str(conn.port)
- if service_name or conn.schema:
- dsn += "/" + (service_name or conn.schema)
+ if service_name:
+ dsn += "/" + service_name
Review comment:
Sorry, I meant to ask why the _change_ was needed, not this entire block. This used to also consider `schema`, but does not anymore. Why not?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #19084: [Oracle] Oracle Hook - automatically set current_schema when defined in Connection
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #19084:
URL: https://github.com/apache/airflow/pull/19084#discussion_r732224449
##########
File path: airflow/providers/oracle/hooks/oracle.py
##########
@@ -136,6 +137,12 @@ def get_conn(self) -> 'OracleHook':
if mod is not None:
conn.module = mod
+ # if Connection.schema is defined, set schema after connecting successfully
+ # cannot be part of conn_config
+ # https://cx-oracle.readthedocs.io/en/latest/api_manual/connection.html?highlight=schema#Connection.current_schema
+ if schema is not None:
+ conn.current_schema = schema
Review comment:
This `current_schema` does not seem to be used anywhere? Why is it needed? It seems to always be the same as `conn.schema`? (Because `schema = conn.schema` on line 80 and it's never modified.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #19084: [Oracle] Oracle Hook - automatically set current_schema when defined in Connection
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #19084:
URL: https://github.com/apache/airflow/pull/19084#discussion_r732225002
##########
File path: airflow/providers/oracle/hooks/oracle.py
##########
@@ -136,6 +137,12 @@ def get_conn(self) -> 'OracleHook':
if mod is not None:
conn.module = mod
+ # if Connection.schema is defined, set schema after connecting successfully
+ # cannot be part of conn_config
+ # https://cx-oracle.readthedocs.io/en/latest/api_manual/connection.html?highlight=schema#Connection.current_schema
+ if schema is not None:
+ conn.current_schema = schema
Review comment:
Oh, never mind, `conn` here is the database connection, not Airflow's `Connection` class. I was confused.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #19084: [Oracle] Oracle Hook - automatically set current_schema when defined in Connection
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #19084:
URL: https://github.com/apache/airflow/pull/19084#issuecomment-1031897211
The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] malthe commented on pull request #19084: [Oracle] Oracle Hook - automatically set current_schema when defined in Connection
Posted by GitBox <gi...@apache.org>.
malthe commented on pull request #19084:
URL: https://github.com/apache/airflow/pull/19084#issuecomment-1038892461
@potiuk this seems to have broken something in our system – just letting you know for release purposes while I investigate.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on pull request #19084: [Oracle] Oracle Hook - automatically set current_schema when defined in Connection
Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #19084:
URL: https://github.com/apache/airflow/pull/19084#issuecomment-1028750856
The linter is complaining.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mehmax commented on a change in pull request #19084: [Oracle] Oracle Hook - automatically set current_schema when defined in Connection
Posted by GitBox <gi...@apache.org>.
mehmax commented on a change in pull request #19084:
URL: https://github.com/apache/airflow/pull/19084#discussion_r778804368
##########
File path: airflow/providers/oracle/hooks/oracle.py
##########
@@ -90,8 +91,19 @@ def get_conn(self) -> 'OracleHook':
dsn = conn.host
if conn.port is not None:
dsn += ":" + str(conn.port)
- if service_name or conn.schema:
- dsn += "/" + (service_name or conn.schema)
+ if service_name:
+ dsn += "/" + service_name
+ elif conn.schema:
+ self.log.warning(
+ """You are using the parameter conn.schema to pass the Service Name
+ of your Oracle Database. In this version of the Hook, conn.schema
+ is additionally used to set the 'current_schema' attribute of the
+ Oracle DB Session.
+ Please use the correct parameter conn.extra.service_name instead to
+ prevent future issues.
+ More Info: https://github.com/apache/airflow/pull/19084"""
+ )
Review comment:
Sure, will look at it!
I also thought of shortening the text a little. Maybe to this:
```
Using conn.schema to pass the Oracle Service Name is deprecated.
Please use conn.extra.service_name instead.
More Info: https://github.com/apache/airflow/pull/19084
```
What do you think about this?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on pull request #19084: [Oracle] Oracle Hook - automatically set current_schema when defined in Connection
Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #19084:
URL: https://github.com/apache/airflow/pull/19084#issuecomment-994466614
Sorry for missing this. This is close to ready IMO.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #19084: [Oracle] Oracle Hook - automatically set current_schema when defined in Connection
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #19084:
URL: https://github.com/apache/airflow/pull/19084#discussion_r732225328
##########
File path: airflow/providers/oracle/hooks/oracle.py
##########
@@ -90,8 +91,8 @@ def get_conn(self) -> 'OracleHook':
dsn = conn.host
if conn.port is not None:
dsn += ":" + str(conn.port)
- if service_name or conn.schema:
- dsn += "/" + (service_name or conn.schema)
+ if service_name:
+ dsn += "/" + service_name
Review comment:
Why is this change needed?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #19084: [Oracle] Oracle Hook - automatically set current_schema when defined in Connection
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #19084:
URL: https://github.com/apache/airflow/pull/19084#issuecomment-1028841779
Yeah . I am really just about to release (slightly delayed) provider's release so it would be great to get that one ien @mehmax
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #19084: [Oracle] Oracle Hook - automatically set current_schema when defined in Connection
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #19084:
URL: https://github.com/apache/airflow/pull/19084#discussion_r798343655
##########
File path: airflow/providers/oracle/hooks/oracle.py
##########
@@ -90,8 +91,8 @@ def get_conn(self) -> 'OracleHook':
dsn = conn.host
if conn.port is not None:
dsn += ":" + str(conn.port)
- if service_name or conn.schema:
- dsn += "/" + (service_name or conn.schema)
+ if service_name:
+ dsn += "/" + service_name
Review comment:
That sounds more user-friendly. Go ahead and make the change please!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org