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 2022/10/27 12:12:52 UTC
[GitHub] [airflow] kolfild26 opened a new pull request, #27319: XCOM push ORA error code in OracleStoredProcedure
kolfild26 opened a new pull request, #27319:
URL: https://github.com/apache/airflow/pull/27319
_**Summary.**_ ORA exit codes are controlled by the DB developer and could be a part of a dag logic. So in order to get access to these codes in dag, I propose an easy enhancement - push ORA exit code to XCOM when DatabaseError occurs.
One file changed - providers/oracle/operators/oracle.py
Motivation and detailed description - https://github.com/apache/airflow/discussions/23787 @potiuk
--
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 merged pull request #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
uranusjr merged PR #27319:
URL: https://github.com/apache/airflow/pull/27319
--
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] bdsoha commented on a diff in pull request #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
bdsoha commented on code in PR #27319:
URL: https://github.com/apache/airflow/pull/27319#discussion_r1015061958
##########
airflow/providers/oracle/operators/oracle.py:
##########
@@ -98,7 +102,9 @@ def execute(self, context: Context):
try:
return hook.callproc(self.procedure, autocommit=True, parameters=self.parameters)
except oracledb.DatabaseError as e:
- code, mesg = e.args[0].message[:-1].split(": ", 1)
- if self.do_xcom_push:
- ti.xcom_push(key="ORA", value=str(code.split("-")[1]))
+ code_match = re.search("^ORA-(\\d+):.+", str(e))
+ if code_match is not None:
+ if self.do_xcom_push:
+ if ti is not None:
+ ti.xcom_push(key="ORA", value=code_match.group(1))
Review Comment:
IMO, this is cognitively complex.
```suggestion
if code_match and self.do_xcom_push and ti:
ti.xcom_push(key="ORA", value=code_match.group(1))
```
--
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] bdsoha commented on pull request #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
bdsoha commented on PR #27319:
URL: https://github.com/apache/airflow/pull/27319#issuecomment-1308753198
Tests are [failing](https://github.com/apache/airflow/actions/runs/3410654132/jobs/5673946111#step:6:2784) due to the lack of a context `Mock`.
https://github.com/apache/airflow/blob/47a2b9ee7f1ff2cc1cc1aa1c3d1b523c88ba29fb/tests/providers/oracle/operators/test_oracle.py#L27-L35
--
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] kolfild26 commented on pull request #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
kolfild26 commented on PR #27319:
URL: https://github.com/apache/airflow/pull/27319#issuecomment-1309231239
`Tests are failing since test_context is a string and not an object.`
that's what I wrote [above](https://github.com/apache/airflow/pull/27319#issuecomment-1305556682).
--
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 diff in pull request #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #27319:
URL: https://github.com/apache/airflow/pull/27319#discussion_r1018817369
##########
airflow/providers/oracle/operators/oracle.py:
##########
@@ -69,6 +72,9 @@ class OracleStoredProcedureOperator(BaseOperator):
:param oracle_conn_id: The :ref:`Oracle connection id <howto/connection:oracle>`
reference to a specific Oracle database.
:param parameters: (optional, templated) the parameters provided in the call
+
+ If *do_xcom_push* is *True*, the numeric exit code emitted by
+ the database is pused to XCom under key ``ORA`` in case of failure.
Review Comment:
I think the intended word is _pushed_.
--
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 #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #27319:
URL: https://github.com/apache/airflow/pull/27319#issuecomment-1313924597
Needstests to be fixed.
--
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 diff in pull request #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #27319:
URL: https://github.com/apache/airflow/pull/27319#discussion_r1015407612
##########
airflow/providers/oracle/operators/oracle.py:
##########
@@ -69,6 +72,9 @@ class OracleStoredProcedureOperator(BaseOperator):
:param oracle_conn_id: The :ref:`Oracle connection id <howto/connection:oracle>`
reference to a specific Oracle database.
:param parameters: (optional, templated) the parameters provided in the call
+
+ :xcom ORA: Oracle db exit code. Optional (if self.do_xcom_push is True).
+ Numeric exit code that Oracle throws in case of failure.
Review Comment:
```suggestion
If *do_xcom_push* is *True*, the numeric exit code emitted by
the database is pused to XCom under key ``ORA`` in case of failure.
```
--
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] eladkal commented on a diff in pull request #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
eladkal commented on code in PR #27319:
URL: https://github.com/apache/airflow/pull/27319#discussion_r1011326774
##########
airflow/providers/oracle/operators/oracle.py:
##########
@@ -93,4 +94,10 @@ def __init__(
def execute(self, context: Context):
self.log.info("Executing: %s", self.procedure)
hook = OracleHook(oracle_conn_id=self.oracle_conn_id)
- return hook.callproc(self.procedure, autocommit=True, parameters=self.parameters)
+ ti=context.get('task_instance')
+ try:
+ return hook.callproc(self.procedure, autocommit=True, parameters=self.parameters)
+ except oracledb.DatabaseError as e:
+ code, mesg = e.args[0].message[:-1].split(': ', 1)
+ ti.xcom_push(key='ORA', value=str(code.split('-')[1]))
Review Comment:
yes
--
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] kolfild26 commented on pull request #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
kolfild26 commented on PR #27319:
URL: https://github.com/apache/airflow/pull/27319#issuecomment-1302144945
@uranusjr
> the various split calls make it pretty difficult to follow what exactly is being extracted from the exception. (Why not just push str(e) and move extraction logic into downstream tasks, for example?)
okaaay... I was actually thinking of these alternatives and previuosly decided to split fully in the operator)
But, okay I could accept that here I relied on too many tiny steps in which smth could be changed in future.
Assume Oracle throws the following:
`ORA-28365: wallet is not open`
Is that acceptable to just split the original output into two parts by `:` and push `ORA-28365` to XCOM β
Doing so, `code.split('-')[1]` will disappear.
The reasons I'd like to cut the message is that
1. It could be long length
2. It's useless in XCOM and, if interested, it's still presented in logs.
I mean `ORA-code` needs for logic, text output needs only for deguging.
--
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 #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on PR #27319:
URL: https://github.com/apache/airflow/pull/27319#issuecomment-1293435574
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
Here are some useful points:
- Pay attention to the quality of your code (flake8, mypy and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/main/STATIC_CODE_CHECKS.rst#prerequisites-for-pre-commit-hooks) will help you with that.
- In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/main/docs/apache-airflow/howto/custom-operator.rst) Consider adding an example DAG that shows how users should use it.
- Consider using [Breeze environment](https://github.com/apache/airflow/blob/main/BREEZE.rst) for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
- Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
- Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
- Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#coding-style-and-best-practices).
Apache Airflow is a community-driven project and together we are making it better π.
In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
Slack: https://s.apache.org/airflow-slack
--
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] kolfild26 commented on a diff in pull request #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
kolfild26 commented on code in PR #27319:
URL: https://github.com/apache/airflow/pull/27319#discussion_r1010836068
##########
airflow/providers/oracle/operators/oracle.py:
##########
@@ -93,4 +94,10 @@ def __init__(
def execute(self, context: Context):
self.log.info("Executing: %s", self.procedure)
hook = OracleHook(oracle_conn_id=self.oracle_conn_id)
- return hook.callproc(self.procedure, autocommit=True, parameters=self.parameters)
+ ti=context.get('task_instance')
+ try:
+ return hook.callproc(self.procedure, autocommit=True, parameters=self.parameters)
+ except oracledb.DatabaseError as e:
+ code, mesg = e.args[0].message[:-1].split(': ', 1)
+ ti.xcom_push(key='ORA', value=str(code.split('-')[1]))
Review Comment:
@eladkal fixed, thanks.
--
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] kolfild26 commented on pull request #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
kolfild26 commented on PR #27319:
URL: https://github.com/apache/airflow/pull/27319#issuecomment-1304960105
1. Changed to re.search() in ora exit parsing
2. description added
> Can you run pre-commit locally and fix linting errors,
3. There were some static check failures since [the last run #step:6:304]( https://github.com/apache/airflow/actions/runs/3372036323/jobs/5602841365 ). Now fixed, locally tested via `breeze static-checks -t run-mypy`:
```
breeze static-checks -t run-mypy --all-files
Checking pre-commit installed for /home/userml/.local/pipx/venvs/apache-airflow-breeze/bin/python
Package pre_commit is installed. Good version 2.20.0 (>= 2.0.0)
Good version of Docker: 20.10.21.
Good version of docker-compose: 2.12.2
Good Docker context used: default.
Run mypy for dev.........................................................Passed
Run mypy for core........................................................Passed
Run mypy for providers...................................................Passed
Run mypy for /docs/ folder...............................................Passed
```
4. u-tests in progress. β³
--
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 diff in pull request #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #27319:
URL: https://github.com/apache/airflow/pull/27319#discussion_r1015387980
##########
airflow/providers/oracle/operators/oracle.py:
##########
@@ -93,4 +99,11 @@ def __init__(
def execute(self, context: Context):
self.log.info("Executing: %s", self.procedure)
hook = OracleHook(oracle_conn_id=self.oracle_conn_id)
- return hook.callproc(self.procedure, autocommit=True, parameters=self.parameters)
+ ti = context.get("task_instance")
+ try:
+ return hook.callproc(self.procedure, autocommit=True, parameters=self.parameters)
+ except oracledb.DatabaseError as e:
+ code_match = re.search("^ORA-(\\d+):.+", str(e))
+ if code_match and self.do_xcom_push and ti:
+ ti.xcom_push(key="ORA", value=code_match.group(1))
Review Comment:
```suggestion
if not self.do_xcom_push or not ti:
raise
code_match = re.search("^ORA-(\\d+):.+", str(e))
if code_match:
ti.xcom_push(key="ORA", value=code_match.group(1))
```
This saves the regex match unless 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] kolfild26 commented on pull request #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
kolfild26 commented on PR #27319:
URL: https://github.com/apache/airflow/pull/27319#issuecomment-1302105673
@uranusjr yes, sure, working on 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] bdsoha commented on pull request #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
bdsoha commented on PR #27319:
URL: https://github.com/apache/airflow/pull/27319#issuecomment-1302687739
>
You can accomplish the same thing using *regex*:
```pythom
import re
code = re.search("^ORA-(\\d+):.+", "ORA-28365: wallet is not open").group(1)
```
--
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] kolfild26 commented on a diff in pull request #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
kolfild26 commented on code in PR #27319:
URL: https://github.com/apache/airflow/pull/27319#discussion_r1010752068
##########
airflow/providers/oracle/operators/oracle.py:
##########
@@ -93,4 +94,10 @@ def __init__(
def execute(self, context: Context):
self.log.info("Executing: %s", self.procedure)
hook = OracleHook(oracle_conn_id=self.oracle_conn_id)
- return hook.callproc(self.procedure, autocommit=True, parameters=self.parameters)
+ ti=context.get('task_instance')
+ try:
+ return hook.callproc(self.procedure, autocommit=True, parameters=self.parameters)
+ except oracledb.DatabaseError as e:
+ code, mesg = e.args[0].message[:-1].split(': ', 1)
+ ti.xcom_push(key='ORA', value=str(code.split('-')[1]))
Review Comment:
@eladkal I just tested, it works, no problem to introduce it, but as it's mentioned in https://github.com/apache/airflow/pull/27370 for many operators, pushing is not optional.
Again, if needed, will add.
--
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] kolfild26 commented on pull request #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
kolfild26 commented on PR #27319:
URL: https://github.com/apache/airflow/pull/27319#issuecomment-1305556682
@bdsoha applied.
@eladkal Some tests failed during [the last run](https://github.com/apache/airflow/actions/runs/3406646740/jobs/5668345263) in `pre-commit run isort`, `pre-commit run run-mypy`, `pre-commit run black`.
I ran locally again. Fixed `isort` and `mypy` errors. `black` did not point to any errors in my env.
Also I ran `breeze testing tests --test-type "Providers[oracle]"` and one existing (**not mine**) test failed:
```
tests/providers/oracle/operators/test_oracle.py:70:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
self = <Task(OracleStoredProcedureOperator): test_task_id>
context = 'test_context'
def execute(self, context: Context):
self.log.info("Executing: %s", self.procedure)
hook = OracleHook(oracle_conn_id=self.oracle_conn_id)
> ti=context.get("task_instance")
AttributeError: 'str' object has no attribute 'get'
```
Here is `context` mocked as `str`. Surely it has no `get` method.
--
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] kolfild26 commented on pull request #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
kolfild26 commented on PR #27319:
URL: https://github.com/apache/airflow/pull/27319#issuecomment-1309350356
@bdsoha
It looks like we could pass `**context` to [test_execute](https://github.com/apache/airflow/blob/47a2b9ee7f1ff2cc1cc1aa1c3d1b523c88ba29fb/tests/providers/oracle/operators/test_oracle.py#L57).
`def test_execute(self, mock_run, **context):`
And then delete [context = "test_context"](https://github.com/apache/airflow/blob/47a2b9ee7f1ff2cc1cc1aa1c3d1b523c88ba29fb/tests/providers/oracle/operators/test_oracle.py#L61)
Tests `breeze testing tests --test-type "Providers[oracle]"` have completed successfully with this change.
I don't know what was the purpose of `context = "test_context"`. It's not used further in test.
--
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 diff in pull request #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #27319:
URL: https://github.com/apache/airflow/pull/27319#discussion_r1007791853
##########
airflow/providers/oracle/operators/oracle.py:
##########
@@ -93,4 +95,10 @@ def __init__(
def execute(self, context: Context):
self.log.info("Executing: %s", self.procedure)
hook = OracleHook(oracle_conn_id=self.oracle_conn_id)
- return hook.callproc(self.procedure, autocommit=True, parameters=self.parameters)
+ ti=context.get('task_instance')
+ try:
+ return hook.callproc(self.procedure, autocommit=True, parameters=self.parameters)
+ except oracledb.DatabaseError as e:
+ code, mesg = e.args[0].message[:-1].split(': ', 1)
+ ti.xcom_push(key='ORA', value=str(code.split('-')[1]))
+ raise AirflowException(f"Task failed with exit code: {code}")
Review Comment:
Instead of coercing the exception to AirflowException, I would prefer this to simply re-raise the original exception:
```suggestion
raise
```
--
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] kolfild26 commented on pull request #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
kolfild26 commented on PR #27319:
URL: https://github.com/apache/airflow/pull/27319#issuecomment-1302677872
@uranusjr
`Can you run pre-commit locally and `
doing this first time, I'm a bit stucked. Could you plz clarify, do these [pre-commit scripts](https://github.com/apache/airflow/tree/main/scripts/ci/pre_commi) only work with [Airflow Breeze docker env](https://github.com/apache/airflow/blob/main/BREEZE.rst) through .yaml scenario?
Or could be run manually one by one? Doesn't look they could be.
--
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] bdsoha commented on a diff in pull request #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
bdsoha commented on code in PR #27319:
URL: https://github.com/apache/airflow/pull/27319#discussion_r1017931926
##########
airflow/providers/oracle/operators/oracle.py:
##########
@@ -69,6 +72,9 @@ class OracleStoredProcedureOperator(BaseOperator):
:param oracle_conn_id: The :ref:`Oracle connection id <howto/connection:oracle>`
reference to a specific Oracle database.
:param parameters: (optional, templated) the parameters provided in the call
+
+ If *do_xcom_push* is *True*, the numeric exit code emitted by
+ the database is pused to XCom under key ``ORA`` in case of failure.
Review Comment:
There is a spelling mistake causing the *build docs* step to fail.
```suggestion
If *do_xcom_push* is *True*, the numeric exit code emitted by
the database is pushed to XCom under key ``ORA`` in case of failure.
```
--
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 #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #27319:
URL: https://github.com/apache/airflow/pull/27319#issuecomment-1339180520
> Hi, I'm a bit stuck with the unit test and will update once it's resolved.
OK. Cool
--
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] kolfild26 commented on pull request #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
kolfild26 commented on PR #27319:
URL: https://github.com/apache/airflow/pull/27319#issuecomment-1345687334
Hi
> I'm a bit stuck with the unit test and will update once it's resolved.
Finally it's done. Please review.
Files modified:
`airflow/providers/oracle/operators/oracle.py`
`tests/providers/oracle/operators/test_oracle.py`
In `test_oracle.py` a new method `test_push_oracle_exit_to_xcom` added.
In `oracle.py` added some check that `context` is available. If so, try to pull from xcom. It's needed as other tests don't use `context` when running `task.execute()`.
--
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] eladkal commented on a diff in pull request #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
eladkal commented on code in PR #27319:
URL: https://github.com/apache/airflow/pull/27319#discussion_r1009286269
##########
airflow/providers/oracle/operators/oracle.py:
##########
@@ -93,4 +94,10 @@ def __init__(
def execute(self, context: Context):
self.log.info("Executing: %s", self.procedure)
hook = OracleHook(oracle_conn_id=self.oracle_conn_id)
- return hook.callproc(self.procedure, autocommit=True, parameters=self.parameters)
+ ti=context.get('task_instance')
+ try:
+ return hook.callproc(self.procedure, autocommit=True, parameters=self.parameters)
+ except oracledb.DatabaseError as e:
+ code, mesg = e.args[0].message[:-1].split(': ', 1)
+ ti.xcom_push(key='ORA', value=str(code.split('-')[1]))
Review Comment:
Should we check for `self.do_xcom_push` ?
--
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] bdsoha commented on pull request #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
bdsoha commented on PR #27319:
URL: https://github.com/apache/airflow/pull/27319#issuecomment-1302688100
> @uranusjr
>
> > the various split calls make it pretty difficult to follow what exactly is being extracted from the exception. (Why not just push str(e) and move extraction logic into downstream tasks, for example?)
>
> okaaay... I was actually thinking of these alternatives and previuosly decided to split fully in the operator)
>
> But, okay I could accept that here I relied on too many tiny steps in which smth could be changed in future.
>
> Assume Oracle throws the following: `ORA-28365: wallet is not open`
>
> Is that acceptable to just split the original output into two parts by `:` and push `ORA-28365` to XCOM β Doing so, `code.split('-')[1]` will disappear.
>
> The reasons I'd like to cut the message is that
>
> 1. It could be long length
> 2. It's useless in XCOM and, if interested, it's still presented in logs.
> I mean `ORA-code` needs for logic, text output only needs for debuging.
You can accomplish the same thing using *regex*:
```pythom
import re
code = re.search("^ORA-(\\d+):.+", "ORA-28365: wallet is not open").group(1)
```
--
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] bdsoha commented on a diff in pull request #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
bdsoha commented on code in PR #27319:
URL: https://github.com/apache/airflow/pull/27319#discussion_r1017931926
##########
airflow/providers/oracle/operators/oracle.py:
##########
@@ -69,6 +72,9 @@ class OracleStoredProcedureOperator(BaseOperator):
:param oracle_conn_id: The :ref:`Oracle connection id <howto/connection:oracle>`
reference to a specific Oracle database.
:param parameters: (optional, templated) the parameters provided in the call
+
+ If *do_xcom_push* is *True*, the numeric exit code emitted by
+ the database is pused to XCom under key ``ORA`` in case of failure.
Review Comment:
There is a spelling mistake causing the *build docs* step to fail.
```suggestion
If *do_xcom_push* is *True*, the numeric exit code emitted by
the database is passed to XCom under key ``ORA`` in case of failure.
```
--
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] kolfild26 commented on a diff in pull request #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
kolfild26 commented on code in PR #27319:
URL: https://github.com/apache/airflow/pull/27319#discussion_r1010693234
##########
airflow/providers/oracle/operators/oracle.py:
##########
@@ -93,4 +94,10 @@ def __init__(
def execute(self, context: Context):
self.log.info("Executing: %s", self.procedure)
hook = OracleHook(oracle_conn_id=self.oracle_conn_id)
- return hook.callproc(self.procedure, autocommit=True, parameters=self.parameters)
+ ti=context.get('task_instance')
+ try:
+ return hook.callproc(self.procedure, autocommit=True, parameters=self.parameters)
+ except oracledb.DatabaseError as e:
+ code, mesg = e.args[0].message[:-1].split(': ', 1)
+ ti.xcom_push(key='ORA', value=str(code.split('-')[1]))
Review Comment:
@eladkal Do you mean checking whether `self.do_xcom_push` is `True` and only if so, then push to XCOM, right?
--
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] kolfild26 commented on a diff in pull request #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
kolfild26 commented on code in PR #27319:
URL: https://github.com/apache/airflow/pull/27319#discussion_r1008286760
##########
airflow/providers/oracle/operators/oracle.py:
##########
@@ -93,4 +95,10 @@ def __init__(
def execute(self, context: Context):
self.log.info("Executing: %s", self.procedure)
hook = OracleHook(oracle_conn_id=self.oracle_conn_id)
- return hook.callproc(self.procedure, autocommit=True, parameters=self.parameters)
+ ti=context.get('task_instance')
+ try:
+ return hook.callproc(self.procedure, autocommit=True, parameters=self.parameters)
+ except oracledb.DatabaseError as e:
+ code, mesg = e.args[0].message[:-1].split(': ', 1)
+ ti.xcom_push(key='ORA', value=str(code.split('-')[1]))
+ raise AirflowException(f"Task failed with exit code: {code}")
Review Comment:
@uranusjr applied. Also removed import. thanks.
--
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] eladkal commented on a diff in pull request #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
eladkal commented on code in PR #27319:
URL: https://github.com/apache/airflow/pull/27319#discussion_r1010765106
##########
airflow/providers/oracle/operators/oracle.py:
##########
@@ -93,4 +94,10 @@ def __init__(
def execute(self, context: Context):
self.log.info("Executing: %s", self.procedure)
hook = OracleHook(oracle_conn_id=self.oracle_conn_id)
- return hook.callproc(self.procedure, autocommit=True, parameters=self.parameters)
+ ti=context.get('task_instance')
+ try:
+ return hook.callproc(self.procedure, autocommit=True, parameters=self.parameters)
+ except oracledb.DatabaseError as e:
+ code, mesg = e.args[0].message[:-1].split(': ', 1)
+ ti.xcom_push(key='ORA', value=str(code.split('-')[1]))
Review Comment:
The flag exist so this could be user choice. The fact that some operators dont use it is not something we should encourage.
--
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] kolfild26 commented on a diff in pull request #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
kolfild26 commented on code in PR #27319:
URL: https://github.com/apache/airflow/pull/27319#discussion_r1010949312
##########
airflow/providers/oracle/operators/oracle.py:
##########
@@ -93,4 +94,10 @@ def __init__(
def execute(self, context: Context):
self.log.info("Executing: %s", self.procedure)
hook = OracleHook(oracle_conn_id=self.oracle_conn_id)
- return hook.callproc(self.procedure, autocommit=True, parameters=self.parameters)
+ ti=context.get('task_instance')
+ try:
+ return hook.callproc(self.procedure, autocommit=True, parameters=self.parameters)
+ except oracledb.DatabaseError as e:
+ code, mesg = e.args[0].message[:-1].split(': ', 1)
+ ti.xcom_push(key='ORA', value=str(code.split('-')[1]))
Review Comment:
@eladkal
> Please add unit test to cover this change
Let me ask, the tests have to be placed in `airflow/tests/providers/oracle/operators/test_oracle.py` right?
--
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] eladkal commented on a diff in pull request #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
eladkal commented on code in PR #27319:
URL: https://github.com/apache/airflow/pull/27319#discussion_r1010914530
##########
airflow/providers/oracle/operators/oracle.py:
##########
@@ -93,4 +94,10 @@ def __init__(
def execute(self, context: Context):
self.log.info("Executing: %s", self.procedure)
hook = OracleHook(oracle_conn_id=self.oracle_conn_id)
- return hook.callproc(self.procedure, autocommit=True, parameters=self.parameters)
+ ti=context.get('task_instance')
+ try:
+ return hook.callproc(self.procedure, autocommit=True, parameters=self.parameters)
+ except oracledb.DatabaseError as e:
+ code, mesg = e.args[0].message[:-1].split(': ', 1)
+ ti.xcom_push(key='ORA', value=str(code.split('-')[1]))
Review Comment:
Consider that not all users need this functionality. Why should we "spam" thier xcom table with data that has no value for them?
The use case of doing something in downstream task with error code of upstream task is not that common. at least from reviewing our issue tracker and questions on slack/other platforms.
--
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] kolfild26 commented on pull request #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
kolfild26 commented on PR #27319:
URL: https://github.com/apache/airflow/pull/27319#issuecomment-1338285436
Hi, I'm a bit stuck with the unit test and will update once it's resolved.
--
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] bdsoha commented on a diff in pull request #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
bdsoha commented on code in PR #27319:
URL: https://github.com/apache/airflow/pull/27319#discussion_r1011551502
##########
airflow/providers/oracle/operators/oracle.py:
##########
@@ -93,4 +94,11 @@ def __init__(
def execute(self, context: Context):
self.log.info("Executing: %s", self.procedure)
hook = OracleHook(oracle_conn_id=self.oracle_conn_id)
- return hook.callproc(self.procedure, autocommit=True, parameters=self.parameters)
+ ti=context.get("task_instance")
+ try:
+ return hook.callproc(self.procedure, autocommit=True, parameters=self.parameters)
+ except oracledb.DatabaseError as e:
+ code, mesg = e.args[0].message[:-1].split(": ", 1)
+ if self.do_xcom_push is True:
Review Comment:
No need to be so explicit
```suggestion
if self.do_xcom_push:
```
--
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] bdsoha commented on a diff in pull request #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
bdsoha commented on code in PR #27319:
URL: https://github.com/apache/airflow/pull/27319#discussion_r1011551502
##########
airflow/providers/oracle/operators/oracle.py:
##########
@@ -93,4 +94,11 @@ def __init__(
def execute(self, context: Context):
self.log.info("Executing: %s", self.procedure)
hook = OracleHook(oracle_conn_id=self.oracle_conn_id)
- return hook.callproc(self.procedure, autocommit=True, parameters=self.parameters)
+ ti=context.get("task_instance")
+ try:
+ return hook.callproc(self.procedure, autocommit=True, parameters=self.parameters)
+ except oracledb.DatabaseError as e:
+ code, mesg = e.args[0].message[:-1].split(": ", 1)
+ if self.do_xcom_push is True:
Review Comment:
```suggestion
if self.do_xcom_push:
```
--
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 #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
uranusjr commented on PR #27319:
URL: https://github.com/apache/airflow/pull/27319#issuecomment-1301852403
Can you run pre-commit locally and fix linting errors, and add a test case for this? Some note in the operator docstring to explain what exactly is pushed to XCom would be nice as well; the various `split` calls make it pretty difficult to follow what exactly is being extracted from the exception. (Why not just push `str(e)` and move extraction logic into downstream tasks, for example?)
--
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] bdsoha commented on a diff in pull request #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
bdsoha commented on code in PR #27319:
URL: https://github.com/apache/airflow/pull/27319#discussion_r1010853681
##########
airflow/providers/oracle/operators/oracle.py:
##########
@@ -93,4 +94,10 @@ def __init__(
def execute(self, context: Context):
self.log.info("Executing: %s", self.procedure)
hook = OracleHook(oracle_conn_id=self.oracle_conn_id)
- return hook.callproc(self.procedure, autocommit=True, parameters=self.parameters)
+ ti=context.get('task_instance')
+ try:
+ return hook.callproc(self.procedure, autocommit=True, parameters=self.parameters)
+ except oracledb.DatabaseError as e:
+ code, mesg = e.args[0].message[:-1].split(': ', 1)
+ ti.xcom_push(key='ORA', value=str(code.split('-')[1]))
Review Comment:
IMO, there is a difference between *data* returned from an operator and *meta-data* describing the operator's execution. The `self.do_xcom_push` controls the storing *(pushing)* of data returned by the logic of a given operator and not *meta-data*.
@eladkal What do you think?
--
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] kolfild26 commented on pull request #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
kolfild26 commented on PR #27319:
URL: https://github.com/apache/airflow/pull/27319#issuecomment-1302713101
@bdsoha
> You can accomplish the same thing using regex:
>
> `import re`
> `code = re.search("^ORA-(\\d+):.+", "ORA-28365: wallet is not open").group(1)`
Yes, but above were the suggestion by @uranusjr not to split so much at all, no matter which way is used:
> Why not just push str(e) and move extraction logic into downstream tasks, for example
I prefer the way @bdsoha (to split). So if no objections to this I would commit `re.search`
--
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 #27319: XCOM push ORA error code in OracleStoredProcedure
Posted by GitBox <gi...@apache.org>.
uranusjr commented on PR #27319:
URL: https://github.com/apache/airflow/pull/27319#issuecomment-1302724327
If we are sure oracledb always emits errors in this format, splitting out the part before `:` makes sense. Using `re` is also a good idea; it is more readable, and itβs easier to catch cases where somehow the format is different (we can simply not push the XCom in this case).
--
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