[AIRFLOW-843] Store exceptions on task_instance #2135
Conversation
|
@thesquelched, thanks for your PR! By analyzing the history of the files in this pull request, we identified @mistercrunch, @bolkedebruin and @saguziel to be potential reviewers. |
|
Tests fail on unrelated LDAP issue; should be good to merge |
|
Just went to Issues to file this exact thing. Happy to see someone's on it! |
thesquelched
force-pushed
the
AIRFLOW-843
branch
from
51fafcd
to
70f98d7
Compare
Codecov Report
@@ Coverage Diff @@
## master #2135 +/- ##
==========================================
+ Coverage 75.92% 75.93% +<.01%
==========================================
Files 199 199
Lines 15954 15956 +2
==========================================
+ Hits 12113 12116 +3
+ Misses 3841 3840 -1
Continue to review full report at Codecov.
|
airflow/models.py
Outdated
| @@ -1574,6 +1574,8 @@ def dry_run(self): | |||
| def handle_failure(self, error, test_mode=False, context=None, session=None): | |||
| self.log.exception(error) | |||
| task = self.task | |||
| session = settings.Session() | |||
There was a problem hiding this comment.
What is this line for? It seems like it does nothing?
There was a problem hiding this comment.
Cruft from testing, I suppose; I'll remove
airflow/models.py
Outdated
| @@ -1574,6 +1574,8 @@ def dry_run(self): | |||
| def handle_failure(self, error, test_mode=False, context=None, session=None): | |||
| self.log.exception(error) | |||
| task = self.task | |||
| session = settings.Session() | |||
| self.exception = error | |||
There was a problem hiding this comment.
Adding a property to the TaskInstance object feels the wrong place for this - the context object feels like a more appropriate place.
|
I've just been lazy; I'll look at this again |
|
@thesquelched, do you have any plans for updating this PR? |
Store exceptions encountered executing a task on the task instance object, making it available for on_failure_callback handlers.
thesquelched
force-pushed
the
AIRFLOW-843
branch
from
70f98d7
to
e584190
Compare
|
@xnuinside sorry for the long wait; rebased and addressed PR comments |
|
@ashb looks good now? |
|
Perfect, thanks! We'll get this in to 1.10.1 |
…2135) Store exceptions encountered executing a task in the context dict, making it available for on_failure_callback handlers.
…pache#2135) Store exceptions encountered executing a task in the context dict, making it available for on_failure_callback handlers.
…pache#2135) Store exceptions encountered executing a task in the context dict, making it available for on_failure_callback handlers.
Store exceptions encountered executing a task in the context dict, making it available for on_failure_callback handlers.
…pache#2135) Store exceptions encountered executing a task in the context dict, making it available for on_failure_callback handlers.
…pache#2135) Store exceptions encountered executing a task in the context dict, making it available for on_failure_callback handlers.
…back (apache#2135) (#28) Store exceptions encountered executing a task in the context dict, making it available for on_failure_callback handlers. Co-authored-by: Scott Kruger <scott@chojin.org>
Store exceptions encountered executing a task on the task instance
object, making it available for on_failure_callback handlers.
Dear Airflow Maintainers,
Please accept this PR that addresses the following issues:
Testing Done: