[GitHub] [airflow] jedcunningham commented on pull request #30259: Add a cache to Variable.get when called at dag parsing time

["jedcunningham (via GitHub)" <gi...@apache.org>]

jedcunningham commented on PR #30259:
URL: https://github.com/apache/airflow/pull/30259#issuecomment-1581964146

   As I posted in this [slack thread](https://apache-airflow.slack.com/archives/C03G9H97MM2/p1681338881105309?thread_ts=1680731048.595729&cid=C03G9H97MM2) (and consolidated/reposted here for posterity in case it rolls off of visible slack history [slightly edited]):
   
   ---
   
   I could break this super easily. I’ve seen enough folks using variables to dictate the structure of their DAG to know this will be problematic. Having parsing be stale is asking for runtime problems.
   
   If we land it, we can’t position it as a “safe” thing to turn on (which is how I read the current docs changes).
   
   Also with many schedulers, you could get them fighting on what the latest DAG looks like. Increasing db write traffic.
   
   Comes back to the [“solution” vs “product” thing](https://medium.com/apache-airflow/magic-loop-in-airflow-reloaded-3e1bd8fb6671). I don’t think this is safe enough to be a product thing, even if it does solve some situations pretty well. 
   
   ---
   
   I really don't think adding caching in only one airflow component is a good idea. I know I don't want to get dragged into support calls about this. Dynamic DAGs are one of the first questions I ask already :)
   


-- 
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