Hey everyone, I hope I'm not late to the discussion. :)
IMO, there is a similarity between Airflow's Setup/Teardown and Pytest's Setup/Teardown, and Pytest allows setup and teardown to exist independently. There are cases where you might need one and not the other. But I'm not sure of any airflow-specific concerns here. Thanks, Utkarsh On Tue, Jul 4, 2023 at 3:45 PM Jarek Potiuk <ja...@potiuk.com> wrote: > > we require that a setup always have a teardown > > I'd personally prefer this one because "explicit is better than implicit". > I see the cases where it could be useful (I was also contemplating no > setup/no teardown before), but I think the wiggly behaviour is a recipe for > a number of mistakes (and issues raised by users), especially when someone > modifies existing DAG and adds teardown without realising the consequences. > > Not a strong preference, I am ok with both, but if I got to choose, I'd > require teardown. > > J. > > > > > On Fri, Jun 30, 2023 at 10:28 PM Daniel Standish > <daniel.stand...@astronomer.io.invalid> wrote: > > > Ok want to pick back up setup / teardown discussion as we get closer to > > 2.7. > > > > For personal reasons I had to take some time off work just as we were > > wrapping up work on 2.6, and at least partly due to that, we punted > setup / > > teardown to 2.7. > > > > But we've picked it back up and continued along the path outlined in this > > thread and things look good for 2.7. > > > > In the time since the previous discussion, not a lot has changed except > > that we've implemented in main some of the things discussed. > > > > For one we added a one-liner syntax. > > > > So now if you have an existing dag such as this: > > > > create_cluster >> run_query >> delete_cluster > > > > You can convert it to use setup / teardown with the following: > > > > create_cluster >> run_query >> > > delete_cluster.as_teardown(setups=create_cluster) > > > > The context manager suggested by Ash has been added, so if you have setup > > `s` and teardown `t`, you can do the following: > > > > with s >> t: > > some_work(other_work()) > > > > Notably, just as with task group context manager, you must instantiate > the > > task inside the context in order for it to be registered. Ephraim is > > working on some improvements to the context manager to address this (so > > that you can explicitly add tasks to the context in that scenario) and so > > that we can throw an error on non-sensical usages such as `with a >> b >> > > c:` > > > > There is a docs PR outlining the current state of the feature here > > <https://github.com/apache/airflow/pull/32169> and you are all welcome > to > > comment. There's still time to make adjustments if desired. > > > > But I wanted to raise one point for discussion here. It's something I > > could have gone either way on, and I want to tap into the wisdom of the > > community. > > > > The question of whether a setup with no teardown, or a teardown with no > > setup, should be added. > > > > There are two options as I see it. > > > > Option 1: if a setup has no teardown, then all the downstreams are > assumed > > to be "in scope" for the setup > > Note: the notion of "scope" is about the clearing behavior aspect of > setup > > / teardown: you clear a task, it clears its setups too. A work task is > in > > the scope of s and t if it's between s and t. > > So then with s1 >> w1 >> w2, clearing work task w1 would clear w1 w2 and > > s1. > > And clearing w2 would clear w2 and s1. > > This makes it easy to have a setup with no teardown. But it makes the > > behavior slightly more... wiggly... for lack of a better term. With this > > example, s1 is a setup for w2. But then suppose you add a teardown after > > w1: s1 >> w1 >> [w2, t1.as_teardown(setups=s1)]. Now s1 is not a setup > for > > w2. > > So the logic is ... "a work task is in scope of a setup if it is in > between > > the setup and its teardown, *or *if the setup has no teardowns". > > > > Option 1: we require that a setup always have a teardown > > With this approach, the rule is simpler and it forces unambiguously > defined > > scopes. If you just want to use a setup and don't need a teardown task, > > you add an empty teardown. The rule is then "a work task is in scope of > a > > setup if it is in between the setup and its teardown" -- i.e. the same > rule > > minus the `or` clause. > > So the example would have to be s1 >> w1 >> w2 >> > > EmptyOperator().as_teardown(setups=s1). > > > > Thanks in advance for your engagement. > > > > P.S. note that those of us in the U.S. will be off for holiday soon so > may > > be slower to respond but just wanted to get this conversation started so > > that we can get this feature to a place that people generally feel good > > enough about in 2.7. > > > > P.P.S. Random-ish note... we should consider marking some features as > beta > > I think. There's a difference between beta and experimental, though > > historically we've only employed the latter. But I think beta can be > good > > to signal "we are committed to this feature we just might make a change > or > > two to the API". Not saying we should do this with setup / teardown > > necessarily but it's an option. Sometimes I think it can be a little > > overwhelming to put out features with the spectre of being locked in by > > backcompat guarantees and perhaps something like a beta feature could be > a > > way to get things out there a little bit more freely and allow for a > > feedback loop. > > > > Thanks again, > > > > Daniel > > >
