Hey Ash,

I personally don't like it, because it is not obvious to me.

Also what happens if you return the `dag_2` variable and set the return value 
in the global context to `dag_2` as well? This is how I used to do it when 
generating DAGs - and in my opinion this is pythonic way of doing it without 
any magic. I mean magic is nice as long as it works..

Keep in mind that also some people use functions to hide the dag creation i.e. 
factory pattern to clearly separate it from callers context (e.g. business 
logic). Your solution would blurry this line.

So I am leaning towards a "No", but keen to know what others think :)

Best,
Felix

Sent with [Proton Mail](https://proton.me/) secure email.

------- Original Message -------
On Tuesday, August 2nd, 2022 at 12:43, Ash Berlin-Taylor <a...@apache.org> 
wrote:

> Hello all,
>
> I'm on a bit of a kick thinking about developer (specifically DAG author) 
> experience and if there is anything we can
>
> Some time ago there was a previous conversation about if we should/could 
> "autoregister" DAGs, rather than just looking at the objects in the top level 
> (globals()) of a file, an I knocked up this PR
>
> https://github.com/apache/airflow/pull/23592
>
> The question I have for you all is do we think this is good idea? It does 
> somewhat subtly change the behaviour in a few cases. Lets take this example 
> this from the docs 
> https://airflow.apache.org/docs/apache-airflow/stable/concepts/dags.html#loading-dags
>
> dag_1
>
> =
>
> DAG
>
> (
>
> 'this_dag_will_be_discovered'
>
> )
>
> def
>
> my_function
>
> ():
>
> dag_2
>
> =
>
> DAG
>
> (
>
> 'but_this_dag_will_not'
>
> )
>
> my_function()
>
> As implemented right now the second dag won't get picked up (as the auto 
> registration is handled in the context manager, but if the example was 
> changed to use a context manager it will get loaded/discovered:
>
> with
>
> DAG
>
> (
>
> 'this_dag_will_be_discovered'
>
> ):
>
> EmptyOperator(task_id='task')
>
> def
>
> my_function
>
> ():
>
> with
>
> DAG
>
> (
>
> 'so_will_this_dag_now'
>
> ):
>
> EmptyOperator(task_id='task')
>
> my_function()
>
> With the change in my PR both DAGs would be picked up. Does that count as a 
> breaking change do you think? Is this behaviour more helpful to users, or do 
> we think it would be confusing?
>
> (If I get a few thumbs up I will update the docs in my PR to cover this new 
> behaviour.)
>
> -ash

Reply via email to