Improve Airflow logging for operator Jinja template processing #25452
Conversation
airflow/models/abstractoperator.py
Outdated
| @@ -77,6 +78,9 @@ | |||
| ) | |||
|
|
|||
|
|
|||
| logger = logging.getLogger("airflow.models.abstractoperator.AbstractOperator") | |||
There was a problem hiding this comment.
Added logger instance, following the same pattern as baseoperator.py.
airflow/models/abstractoperator.py
Outdated
| 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] |
There was a problem hiding this comment.
Work around mypy failures. These issues seemed to be unrelated to my changes.
There was a problem hiding this 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.
There was a problem hiding this 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)
There was a problem hiding this 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
airflow/models/abstractoperator.py
Outdated
| logger.exception( | ||
| f"Exception rendering Jinja template for in " | ||
| f"task '{self.task_id}', field " | ||
| f"'{attr_name}'. Template: {value!r}" | ||
| ) |
There was a problem hiding this 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.
eladkal
changed the title
|
Updated and ready for another review! |
|
Awesome work, congrats on your first merged pull request! |
closes: #25344
Adds logging to provide debugging help for failures in
render_template_fields()^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in newsfragments.