You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "functicons (via GitHub)" <gi...@apache.org> on 2023/11/18 00:55:42 UTC
[PR] KubernetesPodTrigger: add stacktrace to trigger event message [airflow]
functicons opened a new pull request, #35716:
URL: https://github.com/apache/airflow/pull/35716
Ref #34644
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
related: #34644
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
In deferrable mode, the current logs are not sufficient to debug issues happened in the trigger or hook, so adding stack trace and log the event in the operator callback 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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KubernetesPodTrigger: add exception stack trace in TriggerEvent [airflow]
Posted by "functicons (via GitHub)" <gi...@apache.org>.
functicons commented on code in PR #35716:
URL: https://github.com/apache/airflow/pull/35716#discussion_r1406871805
##########
airflow/providers/cncf/kubernetes/operators/pod.py:
##########
@@ -672,6 +672,7 @@ def invoke_defer_method(self):
)
def execute_complete(self, context: Context, event: dict, **kwargs):
+ self.log.info("Triggered with event: %s", event)
Review Comment:
Thanks for reviewing! But I wanted to enable it by default because 1) it is a low frequency message, once per task; 2) when error happens, it is very useful for debugging. I wanted to avoid additional steps to enable debug logs and rerun (sometimes this is not possible when helping customers investigate their prod issues).
Let me know if you still think debug is the right logging level.
--
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
Re: [PR] KubernetesPodTrigger: add exception stack trace in TriggerEvent [airflow]
Posted by "apilaskowski (via GitHub)" <gi...@apache.org>.
apilaskowski commented on PR #35716:
URL: https://github.com/apache/airflow/pull/35716#issuecomment-1838660373
@hussein-awala @jedcunningham can chime in, 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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KubernetesPodTrigger: add exception stack trace in TriggerEvent [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #35716:
URL: https://github.com/apache/airflow/pull/35716
--
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
Re: [PR] KubernetesPodTrigger: add exception stack trace in TriggerEvent [airflow]
Posted by "functicons (via GitHub)" <gi...@apache.org>.
functicons commented on code in PR #35716:
URL: https://github.com/apache/airflow/pull/35716#discussion_r1406871805
##########
airflow/providers/cncf/kubernetes/operators/pod.py:
##########
@@ -672,6 +672,7 @@ def invoke_defer_method(self):
)
def execute_complete(self, context: Context, event: dict, **kwargs):
+ self.log.info("Triggered with event: %s", event)
Review Comment:
Thanks for raising the question! But I wanted to enable it by default because 1) it is a low frequency message, once per task; 2) when error happens, it is very useful for debugging. I wanted to avoid additional steps to enable debug logs and rerun (sometimes this is not possible when helping customers investigate their prod issues).
Let me know if you still think debug is the right logging level.
--
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
Re: [PR] KubernetesPodTrigger: add exception stack trace in TriggerEvent [airflow]
Posted by "functicons (via GitHub)" <gi...@apache.org>.
functicons commented on code in PR #35716:
URL: https://github.com/apache/airflow/pull/35716#discussion_r1406871805
##########
airflow/providers/cncf/kubernetes/operators/pod.py:
##########
@@ -672,6 +672,7 @@ def invoke_defer_method(self):
)
def execute_complete(self, context: Context, event: dict, **kwargs):
+ self.log.info("Triggered with event: %s", event)
Review Comment:
Thanks for raising the question! But I wanted to enable it by default because 1) it is a low frequency message, it is printed at most once per task; 2) when error happens, it is very useful for debugging. I wanted to avoid additional steps to enable debug logs and rerun.
Let me know if you still think debug is the right logging level.
--
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
Re: [PR] KubernetesPodTrigger: add exception stack trace in TriggerEvent [airflow]
Posted by "functicons (via GitHub)" <gi...@apache.org>.
functicons commented on PR #35716:
URL: https://github.com/apache/airflow/pull/35716#issuecomment-1837055012
Friendly ping! Thanks!
--
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
Re: [PR] KubernetesPodTrigger: add exception stack trace in TriggerEvent [airflow]
Posted by "functicons (via GitHub)" <gi...@apache.org>.
functicons commented on code in PR #35716:
URL: https://github.com/apache/airflow/pull/35716#discussion_r1406871805
##########
airflow/providers/cncf/kubernetes/operators/pod.py:
##########
@@ -672,6 +672,7 @@ def invoke_defer_method(self):
)
def execute_complete(self, context: Context, event: dict, **kwargs):
+ self.log.info("Triggered with event: %s", event)
Review Comment:
Thanks for raising the question! But I wanted to enable it by default because 1) it is a low frequency message, at most once per task; 2) when error happens, it is very useful for debugging. I wanted to avoid additional steps to enable debug logs and rerun.
Let me know if you still think debug is the right logging level.
##########
airflow/providers/cncf/kubernetes/operators/pod.py:
##########
@@ -672,6 +672,7 @@ def invoke_defer_method(self):
)
def execute_complete(self, context: Context, event: dict, **kwargs):
+ self.log.info("Triggered with event: %s", event)
Review Comment:
Thanks for raising the question! But I wanted to enable it by default because 1) it is a low frequency message, once per task; 2) when error happens, it is very useful for debugging. I wanted to avoid additional steps to enable debug logs and rerun.
Let me know if you still think debug is the right logging level.
--
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
Re: [PR] KubernetesPodTrigger: add exception stack trace in TriggerEvent [airflow]
Posted by "functicons (via GitHub)" <gi...@apache.org>.
functicons commented on PR #35716:
URL: https://github.com/apache/airflow/pull/35716#issuecomment-1828771225
@jedcunningham and @hussein-awala, could you review this PR? Thank you in advance!
--
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
Re: [PR] KubernetesPodTrigger: add exception stack trace in TriggerEvent [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #35716:
URL: https://github.com/apache/airflow/pull/35716#discussion_r1426903084
##########
airflow/providers/cncf/kubernetes/operators/pod.py:
##########
@@ -672,6 +672,7 @@ def invoke_defer_method(self):
)
def execute_complete(self, context: Context, event: dict, **kwargs):
+ self.log.info("Triggered with event: %s", event)
Review Comment:
Yeah. I thin in this case debug might be a better level, depending on the details printed in the `event` dict - if they are big and long (as I presume) - then such a log entry should definitely be debug.
Suggestion maybe Composer team could introduce some ways of changing the log level by the customer remotely or dynamically when needed ? Actually I think it could be a good idea to add a flag in the UI to do "rerun with debug" - that could be a very nice feature to add to Airflow actually, if Composer tem would find it useful, maybe a good idea to contribute such feature?
--
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
Re: [PR] KubernetesPodTrigger: add exception stack trace in TriggerEvent [airflow]
Posted by "functicons (via GitHub)" <gi...@apache.org>.
functicons commented on code in PR #35716:
URL: https://github.com/apache/airflow/pull/35716#discussion_r1428548938
##########
airflow/providers/cncf/kubernetes/operators/pod.py:
##########
@@ -672,6 +672,7 @@ def invoke_defer_method(self):
)
def execute_complete(self, context: Context, event: dict, **kwargs):
+ self.log.info("Triggered with event: %s", event)
Review Comment:
Updated log level to debug.
--
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
Re: [PR] KubernetesPodTrigger: add exception stack trace in TriggerEvent [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on code in PR #35716:
URL: https://github.com/apache/airflow/pull/35716#discussion_r1405333949
##########
airflow/providers/cncf/kubernetes/operators/pod.py:
##########
@@ -672,6 +672,7 @@ def invoke_defer_method(self):
)
def execute_complete(self, context: Context, event: dict, **kwargs):
+ self.log.info("Triggered with event: %s", event)
Review Comment:
Shouldn't this be debug?
--
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