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/08/01 20:30:35 UTC
[GitHub] [airflow] barrywhart opened a new pull request, #25452: Issue 25344: Improve Airflow logging for operator Jinja template processing
barrywhart opened a new pull request, #25452:
URL: https://github.com/apache/airflow/pull/25452
<!--
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:
closes: #25344
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
Adds logging to provide debugging help for failures in `render_template_fields()`
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an 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 a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
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 a diff in pull request #25452: Issue 25344: Improve Airflow logging for operator Jinja template processing
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #25452:
URL: https://github.com/apache/airflow/pull/25452#discussion_r935129238
##########
airflow/models/abstractoperator.py:
##########
@@ -345,13 +349,22 @@ def _do_render_template_fields(
)
if not value:
continue
- rendered_content = self.render_template(
- value,
- context,
- jinja_env,
- seen_oids,
- )
- setattr(parent, attr_name, rendered_content)
+ try:
+ rendered_content = self.render_template(
+ value,
+ context,
+ jinja_env,
+ seen_oids,
+ )
+ except Exception:
+ logger.exception(
+ f"Exception rendering Jinja template for in "
+ f"task '{self.task_id}', field "
+ f"'{attr_name}'. Template: {value!r}"
+ )
Review Comment:
I think `self.log.exception` is better here; `baseoperator` needed an additional `logger` for free functions outside a class, but this inherits `LoggingMixin` and does not need 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] barrywhart commented on a diff in pull request #25452: Improve Airflow logging for operator Jinja template processing
Posted by GitBox <gi...@apache.org>.
barrywhart commented on code in PR #25452:
URL: https://github.com/apache/airflow/pull/25452#discussion_r935400479
##########
airflow/models/abstractoperator.py:
##########
@@ -391,8 +404,8 @@ def render_template(
template = jinja_env.from_string(value)
dag = self.get_dag()
if dag and dag.render_template_as_native_obj:
- return render_template_as_native(template, context)
- return render_template_to_string(template, context)
+ return render_template_as_native(template, context) # type: ignore[arg-type]
+ return render_template_to_string(template, context) # type: ignore[arg-type]
Review Comment:
I removed the `# type: ignore` comments. We'll see if the CI requires them. Local `mypy` didn't like those lines.
For what it's worth, these are the versions of mypy and related packages in my Breeze Docker environment (newly set up yesterday):
```
mypy==0.950
mypy-boto3-appflow==1.24.36.post1
mypy-boto3-rds==1.24.38
mypy-boto3-redshift-data==1.24.36.post1
mypy-extensions==0.4.3
```
--
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] barrywhart commented on a diff in pull request #25452: Issue 25344: Improve Airflow logging for operator Jinja template processing
Posted by GitBox <gi...@apache.org>.
barrywhart commented on code in PR #25452:
URL: https://github.com/apache/airflow/pull/25452#discussion_r934899704
##########
airflow/models/abstractoperator.py:
##########
@@ -77,6 +78,9 @@
)
+logger = logging.getLogger("airflow.models.abstractoperator.AbstractOperator")
Review Comment:
Added logger instance, following the same pattern as `baseoperator.py`.
--
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] barrywhart commented on a diff in pull request #25452: Improve Airflow logging for operator Jinja template processing
Posted by GitBox <gi...@apache.org>.
barrywhart commented on code in PR #25452:
URL: https://github.com/apache/airflow/pull/25452#discussion_r935385042
##########
airflow/models/abstractoperator.py:
##########
@@ -391,8 +404,8 @@ def render_template(
template = jinja_env.from_string(value)
dag = self.get_dag()
if dag and dag.render_template_as_native_obj:
- return render_template_as_native(template, context)
- return render_template_to_string(template, context)
+ return render_template_as_native(template, context) # type: ignore[arg-type]
+ return render_template_to_string(template, context) # type: ignore[arg-type]
Review Comment:
These are the errors. When you say:
>they need to be fixed first.
Are you requesting that I fix them, or someone else will fix them?
```
airflow/models/abstractoperator.py:402: error: Argument 2 to "render_template_as_native" has incompatible type "Context"; expected "MutableMapping[str, Any]" [arg-type]
return render_template_as_native(template, context)
^
airflow/models/abstractoperator.py:403: error: Argument 2 to "render_template_to_string" has incompatible type "Context"; expected "MutableMapping[str, Any]" [arg-type]
return render_template_to_string(template, context)
```
--
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] boring-cyborg[bot] commented on pull request #25452: Improve Airflow logging for operator Jinja template processing
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on PR #25452:
URL: https://github.com/apache/airflow/pull/25452#issuecomment-1203138180
Awesome work, congrats on your first merged pull request!
--
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 a diff in pull request #25452: Issue 25344: Improve Airflow logging for operator Jinja template processing
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #25452:
URL: https://github.com/apache/airflow/pull/25452#discussion_r935129725
##########
airflow/models/abstractoperator.py:
##########
@@ -391,8 +404,8 @@ def render_template(
template = jinja_env.from_string(value)
dag = self.get_dag()
if dag and dag.render_template_as_native_obj:
- return render_template_as_native(template, context)
- return render_template_to_string(template, context)
+ return render_template_as_native(template, context) # type: ignore[arg-type]
+ return render_template_to_string(template, context) # type: ignore[arg-type]
Review Comment:
What are the errors? We do not want to introduce necessary ignores into the code base; if the errors are indeed unrelated, they need to be fixed first.
--
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] barrywhart commented on a diff in pull request #25452: Issue 25344: Improve Airflow logging for operator Jinja template processing
Posted by GitBox <gi...@apache.org>.
barrywhart commented on code in PR #25452:
URL: https://github.com/apache/airflow/pull/25452#discussion_r934900147
##########
airflow/models/abstractoperator.py:
##########
@@ -391,8 +404,8 @@ def render_template(
template = jinja_env.from_string(value)
dag = self.get_dag()
if dag and dag.render_template_as_native_obj:
- return render_template_as_native(template, context)
- return render_template_to_string(template, context)
+ return render_template_as_native(template, context) # type: ignore[arg-type]
+ return render_template_to_string(template, context) # type: ignore[arg-type]
Review Comment:
Work around `mypy` failures. These issues seemed to be unrelated to my changes.
--
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 merged pull request #25452: Improve Airflow logging for operator Jinja template processing
Posted by GitBox <gi...@apache.org>.
potiuk merged PR #25452:
URL: https://github.com/apache/airflow/pull/25452
--
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] barrywhart commented on pull request #25452: Improve Airflow logging for operator Jinja template processing
Posted by GitBox <gi...@apache.org>.
barrywhart commented on PR #25452:
URL: https://github.com/apache/airflow/pull/25452#issuecomment-1202314822
Updated and ready for another review!
--
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] barrywhart commented on a diff in pull request #25452: Improve Airflow logging for operator Jinja template processing
Posted by GitBox <gi...@apache.org>.
barrywhart commented on code in PR #25452:
URL: https://github.com/apache/airflow/pull/25452#discussion_r935391169
##########
airflow/models/abstractoperator.py:
##########
@@ -345,13 +349,22 @@ def _do_render_template_fields(
)
if not value:
continue
- rendered_content = self.render_template(
- value,
- context,
- jinja_env,
- seen_oids,
- )
- setattr(parent, attr_name, rendered_content)
+ try:
+ rendered_content = self.render_template(
+ value,
+ context,
+ jinja_env,
+ seen_oids,
+ )
+ except Exception:
+ logger.exception(
+ f"Exception rendering Jinja template for in "
+ f"task '{self.task_id}', field "
+ f"'{attr_name}'. Template: {value!r}"
+ )
Review Comment:
Done!
--
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