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/07/27 18:25:52 UTC
[GitHub] [airflow] Jorricks opened a new pull request, #25347: Remove useless logging line
Jorricks opened a new pull request, #25347:
URL: https://github.com/apache/airflow/pull/25347
Currently more than 50% of our scheduler logs are in the form of `Failed to get task ... Marking it as removed`.
There are two of these logging lines in this file, one already has this if else statement, the other one was missing it.
--
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] Jorricks commented on a diff in pull request #25347: Remove useless logging line
Posted by GitBox <gi...@apache.org>.
Jorricks commented on code in PR #25347:
URL: https://github.com/apache/airflow/pull/25347#discussion_r931505899
##########
airflow/models/dagrun.py:
##########
@@ -728,9 +728,12 @@ def _filter_tis_and_exclude_removed(dag: "DAG", tis: List[TI]) -> Iterable[TI]:
try:
ti.task = dag.get_task(ti.task_id)
except TaskNotFound:
- self.log.error("Failed to get task for ti %s. Marking it as removed.", ti)
- ti.state = State.REMOVED
- session.flush()
+ if ti.state == State.REMOVED:
+ pass # ti has already been removed, just ignore it
+ else:
Review Comment:
Yes that seems fair. I think that is also explicit enough indeed.
--
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] Jorricks commented on pull request #25347: Remove useless logging line
Posted by GitBox <gi...@apache.org>.
Jorricks commented on PR #25347:
URL: https://github.com/apache/airflow/pull/25347#issuecomment-1197193835
@eladkal is this still on time for Airflow 2.4.0?
It would be awesome to include this in a `2.3.x` release as well.
--
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] jedcunningham commented on a diff in pull request #25347: Remove useless logging line
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on code in PR #25347:
URL: https://github.com/apache/airflow/pull/25347#discussion_r931492076
##########
airflow/models/dagrun.py:
##########
@@ -728,9 +728,12 @@ def _filter_tis_and_exclude_removed(dag: "DAG", tis: List[TI]) -> Iterable[TI]:
try:
ti.task = dag.get_task(ti.task_id)
except TaskNotFound:
- self.log.error("Failed to get task for ti %s. Marking it as removed.", ti)
- ti.state = State.REMOVED
- session.flush()
+ if ti.state == State.REMOVED:
+ pass # ti has already been removed, just ignore it
+ else:
Review Comment:
```suggestion
if ti.state != State.REMOVED:
```
nit: we can probably just go with 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] potiuk commented on pull request #25347: Remove useless logging line
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #25347:
URL: https://github.com/apache/airflow/pull/25347#issuecomment-1197210458
For 2.4.0 for sure - we have not yet cut the branch for 2.4. I will mark it for 2.3.4 in case we have it, but it's not very likely I think ( we will go straight to 2.4.0 with all bugfixes most likely - but it's up to @jedcunningham and @ephraimbuddy to decided.).
--
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] Jorricks commented on pull request #25347: Remove useless logging line
Posted by GitBox <gi...@apache.org>.
Jorricks commented on PR #25347:
URL: https://github.com/apache/airflow/pull/25347#issuecomment-1197204872
That was quick, even for you @potiuk :p
--
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] jedcunningham merged pull request #25347: Remove useless logging line
Posted by GitBox <gi...@apache.org>.
jedcunningham merged PR #25347:
URL: https://github.com/apache/airflow/pull/25347
--
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