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 2020/01/25 11:54:34 UTC
[GitHub] [airflow] potiuk opened a new pull request #7257: [AIRFLOW-6636]
Avoid exceptions when printing task instance
potiuk opened a new pull request #7257: [AIRFLOW-6636] Avoid exceptions when printing task instance
URL: https://github.com/apache/airflow/pull/7257
Sometimes (mainly in tests) tasks do not have start/execution/end date set and
printing task instance information throws an exception which cover the real
problems and makes diagnostics difficult.
---
Issue link: WILL BE INSERTED BY [boring-cyborg](https://github.com/kaxil/boring-cyborg)
Make sure to mark the boxes below before creating PR: [x]
- [x] Description above provides context of the change
- [x] Commit message/PR title starts with `[AIRFLOW-NNNN]`. AIRFLOW-NNNN = JIRA ID<sup>*</sup>
- [x] Unit tests coverage for changes (not needed for documentation changes)
- [x] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
- [x] Relevant documentation is updated including usage instructions.
- [x] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example).
<sup>*</sup> For document-only changes commit message can start with `[AIRFLOW-XXXX]`.
---
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/master/UPDATING.md).
Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on a change in pull request #7257:
[AIRFLOW-6636] Avoid exceptions when printing task instance
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #7257: [AIRFLOW-6636] Avoid exceptions when printing task instance
URL: https://github.com/apache/airflow/pull/7257#discussion_r370930282
##########
File path: airflow/models/taskinstance.py
##########
@@ -1137,18 +1143,30 @@ def handle_failure(self, error, test_mode=None, context=None, session=None):
'dag_id=%s, task_id=%s, execution_date=%s, start_date=%s, end_date=%s',
self.dag_id,
self.task_id,
- self.execution_date.strftime('%Y%m%dT%H%M%S'),
- self.start_date.strftime('%Y%m%dT%H%M%S'),
- self.end_date.strftime('%Y%m%dT%H%M%S'))
+ self.execution_date.strftime('%Y%m%dT%H%M%S') if hasattr(
+ self,
+ 'execution_date') and self.execution_date else '',
+ self.start_date.strftime('%Y%m%dT%H%M%S') if hasattr(
+ self,
+ 'start_date') and self.start_date else '',
+ self.end_date.strftime('%Y%m%dT%H%M%S') if hasattr(
Review comment:
I think this attribute should always be created. If it is not needed, it should have an empty value. It is better to solve the problem of missing attributes than to hide the problem. All attributes, that the class has, should be created in the __init__ method.
```
self.start_date.strftime('%Y%m%dT%H%M%S') if self.start_date else None,
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on issue #7257: [AIRFLOW-6636] Avoid
exceptions when printing task instance
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #7257: [AIRFLOW-6636] Avoid exceptions when printing task instance
URL: https://github.com/apache/airflow/pull/7257#issuecomment-579747627
Hello here ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on a change in pull request #7257:
[AIRFLOW-6636] Avoid exceptions when printing task instance
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #7257: [AIRFLOW-6636] Avoid exceptions when printing task instance
URL: https://github.com/apache/airflow/pull/7257#discussion_r370930282
##########
File path: airflow/models/taskinstance.py
##########
@@ -1137,18 +1143,30 @@ def handle_failure(self, error, test_mode=None, context=None, session=None):
'dag_id=%s, task_id=%s, execution_date=%s, start_date=%s, end_date=%s',
self.dag_id,
self.task_id,
- self.execution_date.strftime('%Y%m%dT%H%M%S'),
- self.start_date.strftime('%Y%m%dT%H%M%S'),
- self.end_date.strftime('%Y%m%dT%H%M%S'))
+ self.execution_date.strftime('%Y%m%dT%H%M%S') if hasattr(
+ self,
+ 'execution_date') and self.execution_date else '',
+ self.start_date.strftime('%Y%m%dT%H%M%S') if hasattr(
+ self,
+ 'start_date') and self.start_date else '',
+ self.end_date.strftime('%Y%m%dT%H%M%S') if hasattr(
Review comment:
I think this attribute should always be created. If it is not needed, it should have an empty value. It is better to solve the problem of missing attributes than to hide the problem. All attributes, that the class has, should be created in the __init__ method.
```
self.start_date.strftime('%Y%m%dT%H%M%S') if self.start_date else None,
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io commented on issue #7257: [AIRFLOW-6636] Avoid
exceptions when printing task instance
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #7257: [AIRFLOW-6636] Avoid exceptions when printing task instance
URL: https://github.com/apache/airflow/pull/7257#issuecomment-578402860
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7257?src=pr&el=h1) Report
> Merging [#7257](https://codecov.io/gh/apache/airflow/pull/7257?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/ce3635d64f2eba9616709fb9d998676aa34b0875?src=pr&el=desc) will **decrease** coverage by `0.28%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/airflow/pull/7257?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7257 +/- ##
==========================================
- Coverage 85.33% 85.05% -0.29%
==========================================
Files 791 791
Lines 40137 40137
==========================================
- Hits 34252 34137 -115
- Misses 5885 6000 +115
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7257?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/models/taskinstance.py](https://codecov.io/gh/apache/airflow/pull/7257/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvdGFza2luc3RhbmNlLnB5) | `94.53% <ø> (-0.44%)` | :arrow_down: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7257/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7257/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/7257/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/7257/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...viders/cncf/kubernetes/operators/kubernetes\_pod.py](https://codecov.io/gh/apache/airflow/pull/7257/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvY25jZi9rdWJlcm5ldGVzL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZC5weQ==) | `76.47% <0%> (-21.18%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7257?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7257?src=pr&el=footer). Last update [ce3635d...9de1854](https://codecov.io/gh/apache/airflow/pull/7257?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on a change in pull request #7257:
[AIRFLOW-6636] Avoid exceptions when printing task instance
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #7257: [AIRFLOW-6636] Avoid exceptions when printing task instance
URL: https://github.com/apache/airflow/pull/7257#discussion_r370930282
##########
File path: airflow/models/taskinstance.py
##########
@@ -1137,18 +1143,30 @@ def handle_failure(self, error, test_mode=None, context=None, session=None):
'dag_id=%s, task_id=%s, execution_date=%s, start_date=%s, end_date=%s',
self.dag_id,
self.task_id,
- self.execution_date.strftime('%Y%m%dT%H%M%S'),
- self.start_date.strftime('%Y%m%dT%H%M%S'),
- self.end_date.strftime('%Y%m%dT%H%M%S'))
+ self.execution_date.strftime('%Y%m%dT%H%M%S') if hasattr(
+ self,
+ 'execution_date') and self.execution_date else '',
+ self.start_date.strftime('%Y%m%dT%H%M%S') if hasattr(
+ self,
+ 'start_date') and self.start_date else '',
+ self.end_date.strftime('%Y%m%dT%H%M%S') if hasattr(
Review comment:
I think this attribute should always be created. If it is not needed, it should have an empty value. It is better to solve the problem of missing attributes than to hide the problem. All attributes, that the class has, should be created in the __init__ method.
```
self.start_date.strftime('%Y%m%dT%H%M%S') if self.start_date else None,
```
The use of getattr is a trick that allows you to write dynamic code, but we have a known class here that has known attributes. We don't need dynamic objects here, but stable and solid types.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #7257:
[AIRFLOW-6636] Avoid exceptions when printing task instance
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7257: [AIRFLOW-6636] Avoid exceptions when printing task instance
URL: https://github.com/apache/airflow/pull/7257#issuecomment-578402860
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7257?src=pr&el=h1) Report
> Merging [#7257](https://codecov.io/gh/apache/airflow/pull/7257?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/ce3635d64f2eba9616709fb9d998676aa34b0875?src=pr&el=desc) will **decrease** coverage by `0.28%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/airflow/pull/7257?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7257 +/- ##
==========================================
- Coverage 85.33% 85.05% -0.29%
==========================================
Files 791 791
Lines 40137 40137
==========================================
- Hits 34252 34137 -115
- Misses 5885 6000 +115
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7257?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/models/taskinstance.py](https://codecov.io/gh/apache/airflow/pull/7257/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvdGFza2luc3RhbmNlLnB5) | `94.53% <ø> (-0.44%)` | :arrow_down: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7257/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7257/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/7257/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/7257/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...viders/cncf/kubernetes/operators/kubernetes\_pod.py](https://codecov.io/gh/apache/airflow/pull/7257/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvY25jZi9rdWJlcm5ldGVzL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZC5weQ==) | `76.47% <0%> (-21.18%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7257?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7257?src=pr&el=footer). Last update [ce3635d...9de1854](https://codecov.io/gh/apache/airflow/pull/7257?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on a change in pull request #7257:
[AIRFLOW-6636] Avoid exceptions when printing task instance
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #7257: [AIRFLOW-6636] Avoid exceptions when printing task instance
URL: https://github.com/apache/airflow/pull/7257#discussion_r370930282
##########
File path: airflow/models/taskinstance.py
##########
@@ -1137,18 +1143,30 @@ def handle_failure(self, error, test_mode=None, context=None, session=None):
'dag_id=%s, task_id=%s, execution_date=%s, start_date=%s, end_date=%s',
self.dag_id,
self.task_id,
- self.execution_date.strftime('%Y%m%dT%H%M%S'),
- self.start_date.strftime('%Y%m%dT%H%M%S'),
- self.end_date.strftime('%Y%m%dT%H%M%S'))
+ self.execution_date.strftime('%Y%m%dT%H%M%S') if hasattr(
+ self,
+ 'execution_date') and self.execution_date else '',
+ self.start_date.strftime('%Y%m%dT%H%M%S') if hasattr(
+ self,
+ 'start_date') and self.start_date else '',
+ self.end_date.strftime('%Y%m%dT%H%M%S') if hasattr(
Review comment:
I think this attribute should always be created. If it is not needed, it should have an empty value. It is better to solve the problem of missing attributes than to hide the problem. All attributes, that the class has, should be created in the __init__ 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on a change in pull request #7257:
[AIRFLOW-6636] Avoid exceptions when printing task instance
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #7257: [AIRFLOW-6636] Avoid exceptions when printing task instance
URL: https://github.com/apache/airflow/pull/7257#discussion_r370971910
##########
File path: airflow/models/taskinstance.py
##########
@@ -1137,18 +1143,30 @@ def handle_failure(self, error, test_mode=None, context=None, session=None):
'dag_id=%s, task_id=%s, execution_date=%s, start_date=%s, end_date=%s',
self.dag_id,
self.task_id,
- self.execution_date.strftime('%Y%m%dT%H%M%S'),
- self.start_date.strftime('%Y%m%dT%H%M%S'),
- self.end_date.strftime('%Y%m%dT%H%M%S'))
+ self.execution_date.strftime('%Y%m%dT%H%M%S') if hasattr(
+ self,
+ 'execution_date') and self.execution_date else '',
+ self.start_date.strftime('%Y%m%dT%H%M%S') if hasattr(
+ self,
+ 'start_date') and self.start_date else '',
+ self.end_date.strftime('%Y%m%dT%H%M%S') if hasattr(
Review comment:
Can you indicate which test has this problem? It seems to me that the test in this place may fail only in one situation - when execution_date == None, because the existence of the attribute is guaranteed by metaclase - SQlAlchemy.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on a change in pull request #7257:
[AIRFLOW-6636] Avoid exceptions when printing task instance
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #7257: [AIRFLOW-6636] Avoid exceptions when printing task instance
URL: https://github.com/apache/airflow/pull/7257#discussion_r370930282
##########
File path: airflow/models/taskinstance.py
##########
@@ -1137,18 +1143,30 @@ def handle_failure(self, error, test_mode=None, context=None, session=None):
'dag_id=%s, task_id=%s, execution_date=%s, start_date=%s, end_date=%s',
self.dag_id,
self.task_id,
- self.execution_date.strftime('%Y%m%dT%H%M%S'),
- self.start_date.strftime('%Y%m%dT%H%M%S'),
- self.end_date.strftime('%Y%m%dT%H%M%S'))
+ self.execution_date.strftime('%Y%m%dT%H%M%S') if hasattr(
+ self,
+ 'execution_date') and self.execution_date else '',
+ self.start_date.strftime('%Y%m%dT%H%M%S') if hasattr(
+ self,
+ 'start_date') and self.start_date else '',
+ self.end_date.strftime('%Y%m%dT%H%M%S') if hasattr(
Review comment:
I think this attribute should always be created. If it is not needed, it should have an empty value. It is better to solve the problem of missing attributes than to hide the problem. All attributes, that the class has, should be created in the __init__ method.
```python
self.start_date.strftime('%Y%m%dT%H%M%S') if self.start_date else None,
```
The use of getattr is a trick that allows you to write dynamic code, but we have a known class here that has known attributes. We don't need dynamic objects here, but stable and solid types.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #7257:
[AIRFLOW-6636] Avoid exceptions when printing task instance
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #7257: [AIRFLOW-6636] Avoid exceptions when printing task instance
URL: https://github.com/apache/airflow/pull/7257#discussion_r370990274
##########
File path: airflow/models/taskinstance.py
##########
@@ -1137,18 +1143,30 @@ def handle_failure(self, error, test_mode=None, context=None, session=None):
'dag_id=%s, task_id=%s, execution_date=%s, start_date=%s, end_date=%s',
self.dag_id,
self.task_id,
- self.execution_date.strftime('%Y%m%dT%H%M%S'),
- self.start_date.strftime('%Y%m%dT%H%M%S'),
- self.end_date.strftime('%Y%m%dT%H%M%S'))
+ self.execution_date.strftime('%Y%m%dT%H%M%S') if hasattr(
+ self,
+ 'execution_date') and self.execution_date else '',
+ self.start_date.strftime('%Y%m%dT%H%M%S') if hasattr(
+ self,
+ 'start_date') and self.start_date else '',
+ self.end_date.strftime('%Y%m%dT%H%M%S') if hasattr(
Review comment:
This is the error I originally had. The test was still passing and the ERROR sigterm logs were expected (on kill). But then apparently in signal processing path task instance was being printed to log and it was producing this misleading exceptions about NoneType not having strftime. I looked at other places where task instance was printed and I simply aligned all the places to be the same (with getattr and checking for empty) - just to have a consistent approach.
If you can modify all the places to not use getattra and maybe even extract taskinstance printing to separate method and trace down where the attributes could have been missing and make sure it is all tested for all cases, I am happy if you do it.
Note that you should do it for both master and v1-10-test because primary problem was in 1.10 branch.
```
tests/task/task_runner/test_standard_task_runner.py::TestStandardTaskRunner::test_on_kill ========================= AIRFLOW ==========================
Airflow home /root/airflow
Home of the user: /root
Skipping initializing of the DB as it was initialized already.
You can re-initialize the database by adding --with-db-init flag when running tests.
[2020-01-25 11:38:56,950] {{__init__.py:51}} INFO - Using executor SequentialExecutor
[2020-01-25 11:38:56,951] {{dagbag.py:403}} INFO - Filling up the DagBag from /opt/airflow/tests/dags
[2020-01-25 11:38:57,050] {test_task_view_type_check.py:50} INFO - class_instance type: <class 'unusual_prefix_5d280a9b385120fec3c40cfe5be04e2f41b6b5e8_test_task_view_type_check.CallableClass'>
[2020-01-25 11:38:57,085] {{dagbag.py:271}} INFO - File /opt/airflow/tests/dags/test_zip.zip assumed to contain no DAGs. Skipping.
[2020-01-25 11:38:57,144] {{standard_task_runner.py:52}} INFO - Started process 235 to run task
[2020-01-25 11:38:57,196] {{dagbag.py:403}} INFO - Filling up the DagBag from /opt/airflow/tests/dags/test_on_kill.py
Running %s on host %s <TaskInstance: test_on_kill.task1 2016-01-01T00:00:00+00:00 [None]> 7f8368b91b67
[2020-01-25 11:39:00,161] {{helpers.py:322}} INFO - Sending Signals.SIGTERM to GPID 235
[2020-01-25 11:39:00,162] {{taskinstance.py:941}} ERROR - Received SIGTERM. Terminating subprocesses.
[2020-01-25 11:39:00,164] {{test_on_kill.py:36}} INFO - Executing on_kill
[2020-01-25 11:39:00,162] {{taskinstance.py:941}} ERROR - Received SIGTERM. Terminating subprocesses.
[2020-01-25 11:39:00,165] {{test_on_kill.py:36}} INFO - Executing on_kill
[2020-01-25 11:39:00,185] {{taskinstance.py:1122}} ERROR - Task received SIGTERM signal
Traceback (most recent call last):
File "/opt/airflow/airflow/models/taskinstance.py", line 966, in _run_raw_task
result = task_copy.execute(context=context)
File "/opt/airflow/tests/dags/test_on_kill.py", line 32, in execute
os.system('sleep 10')
File "/opt/airflow/airflow/models/taskinstance.py", line 943, in signal_handler
raise AirflowException("Task received SIGTERM signal")
airflow.exceptions.AirflowException: Task received SIGTERM signal
[2020-01-25 11:39:00,185] {{taskinstance.py:1122}} ERROR - Task received SIGTERM signal
Traceback (most recent call last):
File "/opt/airflow/airflow/models/taskinstance.py", line 966, in _run_raw_task
result = task_copy.execute(context=context)
File "/opt/airflow/tests/dags/test_on_kill.py", line 33, in execute
time.sleep(10)
File "/opt/airflow/airflow/models/taskinstance.py", line 943, in signal_handler
raise AirflowException("Task received SIGTERM signal")
airflow.exceptions.AirflowException: Task received SIGTERM signal
[2020-01-25 11:39:00,194] {{taskinstance.py:1171}} ERROR - Failed to send email to: None
[2020-01-25 11:39:00,194] {{taskinstance.py:1171}} ERROR - Failed to send email to: None
[2020-01-25 11:39:00,201] {{taskinstance.py:1172}} ERROR - 'NoneType' object has no attribute 'strftime'
Traceback (most recent call last):
File "/opt/airflow/airflow/models/taskinstance.py", line 966, in _run_raw_task
result = task_copy.execute(context=context)
File "/opt/airflow/tests/dags/test_on_kill.py", line 33, in execute
time.sleep(10)
File "/opt/airflow/airflow/models/taskinstance.py", line 943, in signal_handler
raise AirflowException("Task received SIGTERM signal")
airflow.exceptions.AirflowException: Task received SIGTERM signal
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/opt/airflow/airflow/models/taskinstance.py", line 1166, in handle_failure
self.start_date.strftime('%Y%m%dT%H%M%S'),
AttributeError: 'NoneType' object has no attribute 'strftime'
[2020-01-25 11:39:00,201] {{taskinstance.py:1172}} ERROR - 'NoneType' object has no attribute 'strftime'
Traceback (most recent call last):
File "/opt/airflow/airflow/models/taskinstance.py", line 966, in _run_raw_task
result = task_copy.execute(context=context)
File "/opt/airflow/tests/dags/test_on_kill.py", line 32, in execute
os.system('sleep 10')
File "/opt/airflow/airflow/models/taskinstance.py", line 943, in signal_handler
raise AirflowException("Task received SIGTERM signal")
airflow.exceptions.AirflowException: Task received SIGTERM signal
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/opt/airflow/airflow/models/taskinstance.py", line 1166, in handle_failure
self.start_date.strftime('%Y%m%dT%H%M%S'),
AttributeError: 'NoneType' object has no attribute 'strftime'
[2020-01-25 11:39:00,257] {{helpers.py:288}} INFO - Process psutil.Process(pid=235, status='terminated') (235) terminated with exit code 1
[2020-01-25 11:39:00,258] {{helpers.py:288}} INFO - Process psutil.Process(pid=238, status='terminated') (238) terminated with exit code None
[2020-01-25 11:39:00,259] {{helpers.py:288}} INFO - Process psutil.Process(pid=237, status='terminated') (237) terminated with exit code None
[2020-01-25 11:39:00,288] {{helpers.py:288}} INFO - Process psutil.Process(pid=236, status='terminated') (236) terminated with exit code None
PASSED
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #7257:
[AIRFLOW-6636] Avoid exceptions when printing task instance
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #7257: [AIRFLOW-6636] Avoid exceptions when printing task instance
URL: https://github.com/apache/airflow/pull/7257#discussion_r370989926
##########
File path: airflow/models/taskinstance.py
##########
@@ -1137,18 +1143,30 @@ def handle_failure(self, error, test_mode=None, context=None, session=None):
'dag_id=%s, task_id=%s, execution_date=%s, start_date=%s, end_date=%s',
self.dag_id,
self.task_id,
- self.execution_date.strftime('%Y%m%dT%H%M%S'),
- self.start_date.strftime('%Y%m%dT%H%M%S'),
- self.end_date.strftime('%Y%m%dT%H%M%S'))
+ self.execution_date.strftime('%Y%m%dT%H%M%S') if hasattr(
+ self,
+ 'execution_date') and self.execution_date else '',
+ self.start_date.strftime('%Y%m%dT%H%M%S') if hasattr(
+ self,
+ 'start_date') and self.start_date else '',
+ self.end_date.strftime('%Y%m%dT%H%M%S') if hasattr(
Review comment:
I saw it in a few tests, but one particularly that I could see it happening was:
`pytest tests/task/task_runner/test_standard_task_runner.py -s`
You can run it in v1-10-test branch (commit ca0bbba19add1aef564c3379a9b8dfce55000cf1) to reproduce it. I have already applied this change with formatting while testing v1-10-test branch so it does not occur in the v1-10-test branch tip.
I am now trying to fix all the flakiness and make v1-10-test stable and remove all the false information printed/flaky tests etc. so if you would like to spend quite some time and investigate it and make sure that it's fixed better - I am happy if you can help with this tasks.
Just to give you perspective - I had quite a lot to do to fix all the various test flakiness so I have no time to analyse all the path of TaskInstance printing and adding if's is the easiest one so I do not have think about this as well. So if you have time and can do it better I am more than happy. I assigned it to you then and maybe we can fix it together: https://issues.apache.org/jira/browse/AIRFLOW-6640
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] potiuk merged pull request #7257: [AIRFLOW-6636] Avoid
exceptions when printing task instance
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #7257: [AIRFLOW-6636] Avoid exceptions when printing task instance
URL: https://github.com/apache/airflow/pull/7257
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #7257:
[AIRFLOW-6636] Avoid exceptions when printing task instance
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #7257: [AIRFLOW-6636] Avoid exceptions when printing task instance
URL: https://github.com/apache/airflow/pull/7257#discussion_r370990340
##########
File path: airflow/models/taskinstance.py
##########
@@ -1137,18 +1143,30 @@ def handle_failure(self, error, test_mode=None, context=None, session=None):
'dag_id=%s, task_id=%s, execution_date=%s, start_date=%s, end_date=%s',
self.dag_id,
self.task_id,
- self.execution_date.strftime('%Y%m%dT%H%M%S'),
- self.start_date.strftime('%Y%m%dT%H%M%S'),
- self.end_date.strftime('%Y%m%dT%H%M%S'))
+ self.execution_date.strftime('%Y%m%dT%H%M%S') if hasattr(
+ self,
+ 'execution_date') and self.execution_date else '',
+ self.start_date.strftime('%Y%m%dT%H%M%S') if hasattr(
+ self,
+ 'start_date') and self.start_date else '',
+ self.end_date.strftime('%Y%m%dT%H%M%S') if hasattr(
Review comment:
Let me know how you want to proceed 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #7257:
[AIRFLOW-6636] Avoid exceptions when printing task instance
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #7257: [AIRFLOW-6636] Avoid exceptions when printing task instance
URL: https://github.com/apache/airflow/pull/7257#discussion_r370957648
##########
File path: airflow/models/taskinstance.py
##########
@@ -1137,18 +1143,30 @@ def handle_failure(self, error, test_mode=None, context=None, session=None):
'dag_id=%s, task_id=%s, execution_date=%s, start_date=%s, end_date=%s',
self.dag_id,
self.task_id,
- self.execution_date.strftime('%Y%m%dT%H%M%S'),
- self.start_date.strftime('%Y%m%dT%H%M%S'),
- self.end_date.strftime('%Y%m%dT%H%M%S'))
+ self.execution_date.strftime('%Y%m%dT%H%M%S') if hasattr(
+ self,
+ 'execution_date') and self.execution_date else '',
+ self.start_date.strftime('%Y%m%dT%H%M%S') if hasattr(
+ self,
+ 'start_date') and self.start_date else '',
+ self.end_date.strftime('%Y%m%dT%H%M%S') if hasattr(
Review comment:
I agree in principle. It should be as you described. But currently we face different reality. Solving it in the way you describe is much bigger task.
I just want to solve current tests not to show failures where there are no failures at all. Failing exception in "log" is not a good "test" so it does not hide the problem. Logs should never "detect" a problem - they should print the current state of the object and not randomly fail.
Right now in logs you see unrelated failures, you have inconsistency in the way how those attributes are treated - and sometimes you think those are the problems with tests - where they aren't.
So right now I am not trying to solve this "bigger" problem only the small one. But I created
the issue and marked you there https://issues.apache.org/jira/browse/AIRFLOW-6640 to solve the bigger 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services