[GitHub] [airflow] mik-laj commented on a change in pull request #7257: [AIRFLOW-6636] Avoid exceptions when printing task instance
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:
us...@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
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:
us...@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
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:
us...@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
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:
us...@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
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:
us...@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
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:
us...@infra.apache.org
With regards,
Apache Git Services
