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/06 18:07:40 UTC

[GitHub] [airflow] malthe opened a new pull request #20729: Avoid duplicate logging

malthe opened a new pull request #20729:
URL: https://github.com/apache/airflow/pull/20729


   The error is already logged elsewhere.
   
   (Both in the case of a string message and an exception.)
   
   See also https://github.com/apache/airflow/discussions/20060 where it is also mentioned that exceptions are logged twice during normal Airflow task execution:
   
   1. In `taskinstance` (which is removed with this pull request)
   2. In `standard_task_runner`
   
   It is thus the responsibility of the calling framework to log the exception or error.


-- 
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 pull request #20729: Avoid duplicate logging

Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #20729:
URL: https://github.com/apache/airflow/pull/20729#issuecomment-1008605322


   IMO that’s putting to much responsibility on the task runner implementation. The task instance errors, so the task instance logs it. If that’s undesired, overwrite the task instance’s logger. The runner shouldn’t be involved.


-- 
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 closed pull request #20729: Avoid duplicate logging

Posted by GitBox <gi...@apache.org>.
malthe closed pull request #20729:
URL: https://github.com/apache/airflow/pull/20729


   


-- 
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 pull request #20729: Avoid duplicate logging

Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #20729:
URL: https://github.com/apache/airflow/pull/20729#issuecomment-1008512936


   But if the task is run by a custom task runner (that does not inherit from `StandardTaskRunner`), would this not potentially cause the error to not be logged at all? Maybe we should remove the log in `StandardTaskRunner` instead?


-- 
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 #20729: Avoid duplicate logging

Posted by GitBox <gi...@apache.org>.
malthe commented on pull request #20729:
URL: https://github.com/apache/airflow/pull/20729#issuecomment-1008982354


   This work has been folded into #20731.


-- 
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 #20729: Avoid duplicate logging

Posted by GitBox <gi...@apache.org>.
malthe commented on pull request #20729:
URL: https://github.com/apache/airflow/pull/20729#issuecomment-1008588878


   @uranusjr yes that does make sense although this of course removes the ability of the task runner to do its own thing with regards to logging exceptions.


-- 
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 edited a comment on pull request #20729: Avoid duplicate logging

Posted by GitBox <gi...@apache.org>.
malthe edited a comment on pull request #20729:
URL: https://github.com/apache/airflow/pull/20729#issuecomment-1008982354


   This work has been folded into #20731 – such that the exception handling now happens in the task instance execution logic instead of the task runner.


-- 
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