potiuk commented on PR #35546:
URL: https://github.com/apache/airflow/pull/35546#issuecomment-1803544286
> > For example, our team heavily relies on templating this field (e.g., "{{
var.value.some_email_list }}") as the default_args["email"] for each and every
DAG we have, and we don't want to use Variable.get("some_email_list") on the
module level of our scripts instead because it's actually an anti-pattern
(opening a DB connection and making a SQL query under the hood at parsing time
and not at execution time).
>
> @gdavoian No strong objections here, but I'm a little concerned that if it
is a specific case of your team then the downside of this is we add an
[overhead](https://github.com/apache/airflow/blob/9356233762a67c0a787fc3e16040455019af13bd/airflow/models/abstractoperator.py#L674)
of rendering this field for all operators in airflow for everyone who uses it.
>
> Also, I checked the
[code](https://github.com/apache/airflow/blob/main/airflow/models/variable.py#L271),
and `Variable.get()` is using a cache so it might not be a bad idea to use it,
considering the downside. WDYT?
Yeah. It looks like a useful case - mostly because of the specific place
where it is used.
Actually this case is I think largerly superseeded by Notifiers, but having
this specific exception for email does not seem to have a side-effect, and I
can easily imagine how it might be useful. We've been discussing the reasoning
for that with @gdavoian and @hussein-awala in
https://github.com/apache/airflow/pull/29821#issuecomment-1802925162 and it
seems like it would not hurt to add 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-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
