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/01/29 21:19:37 UTC
[GitHub] [airflow] malthe opened a new pull request #21213: Log traceback in trigger excs
malthe opened a new pull request #21213:
URL: https://github.com/apache/airflow/pull/21213
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
If an exception is raised in a triggerer job during (asynchronous) execution of a trigger, the exception is logged locally, but the traceback is not sent to the task log.
Instead, the fail-handler simply logs a message that the trigger has failed.
---
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/main/UPDATING.md).
--
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 a change in pull request #21213: Log traceback in trigger excs
Posted by GitBox <gi...@apache.org>.
malthe commented on a change in pull request #21213:
URL: https://github.com/apache/airflow/pull/21213#discussion_r795965918
##########
File path: tests/jobs/test_triggerer_job.py
##########
@@ -448,7 +452,7 @@ def test_invalid_trigger(session, dag_maker):
job.load_triggers()
# Make sure it turned up in the failed queue
- assert list(job.runner.failed_triggers) == [1]
+ assert len(job.runner.failed_triggers) == 1
Review comment:
Perhaps – but all tests are heuristics I suppose. 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] malthe commented on pull request #21213: Log traceback in trigger excs
Posted by GitBox <gi...@apache.org>.
malthe commented on pull request #21213:
URL: https://github.com/apache/airflow/pull/21213#issuecomment-1054732439
@eladkal got time to check up on this one?
--
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 #21213: Log traceback in trigger excs
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #21213:
URL: https://github.com/apache/airflow/pull/21213#discussion_r795934351
##########
File path: tests/jobs/test_triggerer_job.py
##########
@@ -448,7 +452,7 @@ def test_invalid_trigger(session, dag_maker):
job.load_triggers()
# Make sure it turned up in the failed queue
- assert list(job.runner.failed_triggers) == [1]
+ assert len(job.runner.failed_triggers) == 1
Review comment:
Should we still check `job.runner.failed_triggers[0][0]` is `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] github-actions[bot] commented on pull request #21213: Log traceback in trigger excs
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #21213:
URL: https://github.com/apache/airflow/pull/21213#issuecomment-1026061088
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, 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] uranusjr merged pull request #21213: Log traceback in trigger excs
Posted by GitBox <gi...@apache.org>.
uranusjr merged pull request #21213:
URL: https://github.com/apache/airflow/pull/21213
--
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 #21213: Log traceback in trigger excs
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #21213:
URL: https://github.com/apache/airflow/pull/21213#discussion_r796382498
##########
File path: tests/jobs/test_triggerer_job.py
##########
@@ -448,7 +452,7 @@ def test_invalid_trigger(session, dag_maker):
job.load_triggers()
# Make sure it turned up in the failed queue
- assert list(job.runner.failed_triggers) == [1]
+ assert len(job.runner.failed_triggers) == 1
Review comment:
```python
assert len(job.runner.failed_triggers) == 1
assert job.runner.failed_triggers[0][0] == 1
```
Is good enough 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] dstandish commented on a change in pull request #21213: Log traceback in trigger excs
Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #21213:
URL: https://github.com/apache/airflow/pull/21213#discussion_r795991779
##########
File path: tests/jobs/test_triggerer_job.py
##########
@@ -448,7 +452,7 @@ def test_invalid_trigger(session, dag_maker):
job.load_triggers()
# Make sure it turned up in the failed queue
- assert list(job.runner.failed_triggers) == [1]
+ assert len(job.runner.failed_triggers) == 1
Review comment:
isn't the before test stricter?
##########
File path: airflow/models/taskinstance.py
##########
@@ -1482,6 +1482,9 @@ def _execute_task(self, context, task_copy):
# this task was scheduled specifically to fail.
if self.next_method == "__fail__":
next_kwargs = self.next_kwargs or {}
+ traceback = self.next_kwargs.get("traceback")
+ if traceback is not None:
+ self.log.error("Trigger failed:\n%s", "\n".join(traceback))
Review comment:
```suggestion
self.log.error("Trigger failed:\n%s", "".join(traceback))
```
when i tested this locally, the lines already were newline-terminated
--
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 a change in pull request #21213: Log traceback in trigger excs
Posted by GitBox <gi...@apache.org>.
malthe commented on a change in pull request #21213:
URL: https://github.com/apache/airflow/pull/21213#discussion_r796011533
##########
File path: tests/jobs/test_triggerer_job.py
##########
@@ -448,7 +452,7 @@ def test_invalid_trigger(session, dag_maker):
job.load_triggers()
# Make sure it turned up in the failed queue
- assert list(job.runner.failed_triggers) == [1]
+ assert len(job.runner.failed_triggers) == 1
Review comment:
Yeah, the before test was stricter. But now failed triggers are a 2-tuple so if we test the first component, do we then need to test the other one as well etc – to my mind, these tests are just heuristics trying to help us prevent bugs, but they can't be exhaustive.
But I can make it as strict as before.
--
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 #21213: Log traceback in trigger excs
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #21213:
URL: https://github.com/apache/airflow/pull/21213#issuecomment-1030901828
Can you please rebase @malthe - we fixed a few issues in main.
--
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 a change in pull request #21213: Log traceback in trigger excs
Posted by GitBox <gi...@apache.org>.
malthe commented on a change in pull request #21213:
URL: https://github.com/apache/airflow/pull/21213#discussion_r796667434
##########
File path: tests/jobs/test_triggerer_job.py
##########
@@ -448,7 +452,7 @@ def test_invalid_trigger(session, dag_maker):
job.load_triggers()
# Make sure it turned up in the failed queue
- assert list(job.runner.failed_triggers) == [1]
+ assert len(job.runner.failed_triggers) == 1
Review comment:
Fixed in c891e106905a864c3aa5da57cd8495449e4d3b65.
##########
File path: tests/jobs/test_triggerer_job.py
##########
@@ -448,7 +452,7 @@ def test_invalid_trigger(session, dag_maker):
job.load_triggers()
# Make sure it turned up in the failed queue
- assert list(job.runner.failed_triggers) == [1]
+ assert len(job.runner.failed_triggers) == 1
Review comment:
Fixed in c891e106905a864c3aa5da57cd8495449e4d3b65.
--
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