tatiana commented on code in PR #30943:
URL: https://github.com/apache/airflow/pull/30943#discussion_r1182288812


##########
airflow/models/abstractoperator.py:
##########
@@ -564,7 +566,7 @@ def _do_render_template_fields(
                     f"{attr_name!r} is configured as a template field "
                     f"but {parent.task_type} does not have this attribute."
                 )
-            if not value:
+            if isinstance(value, pd.DataFrame) or not value:

Review Comment:
   I followed @dstandish 's recommendation on removing the dependency on Pandas 
DataFrame. I'm sorry, I hadn't realised it wasn't a core Airflow dependency!
   
   I  also implemented @potiuk's suggestion on capturing the `ValueError` 
exception, as opposed to the `empty` suggestion.  There were two motivations 
for this:
   
   1. Let's say `_do_render_template_fields` receives a non-empty `Pandas` 
`DataFrame`. Does Airflow have the capability of rendering Jinja in DataFrames? 
Wouldn't it make more sense to `continue`?
   
   2. It feels more resilient since it works with classes which do not 
implement the `empty` 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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to