[GitHub] [airflow] malthe commented on pull request #23592: Automatically register DAGs that are used in a context manager

[GitBox <gi...@apache.org>]

malthe commented on PR #23592:
URL: https://github.com/apache/airflow/pull/23592#issuecomment-1121317236

   @ashb there are at least two things to consider here which I stumbled upon playing around with this:
   
   1. There's nothing special about `with` except that tasks are automatically assigned to this dag (i.e., `dag=dag`). Tying registration of DAGs to `with` is perhaps a bit confusing in this sense – also, you can use `with` on a particular dag instance multiple times!
   
   2. What about DAGs declared in functions?
   
       ```python
       def test():
           with DAG():
               ...
       test()
       ```
   
       I don't mind but it is a little magic perhaps, that this automatically _declares_ the DAG.
   
   I ended up instead with this pattern:
   ```python
   dag = DAG(...)
   
   with dag:
       ...
   ```
   – as a recommended pattern, simply because then you can't forget to give the DAG instance a top-level variable.
   
   And then as I mention in https://lists.apache.org/thread/xndt3zklxbzo5nlmjz10dtfm2pp4t4wq, we could instead warn the user if they'd created a DAG which isn't exposed/registered.
   
   (The gist of that linked reply is that there are two different ways to do that.)


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