dstandish commented on PR #32053: URL: https://github.com/apache/airflow/pull/32053#issuecomment-1601472729
> > > This looks good but what do you think about not having `as_teardown(setuptask)`? > > > > > > What don't you like about it? > > Would you prefer kwargs only? > > Or perhaps a separate method such as `with_setup`? Or `teardown_for`? But just seems a bit redundant. > > I was thinking that `with s1 >> t1` should suffix. Makes the syntax similar to what we have with the decorators > > Hmm. Looks like there's a bug with classic operator setup/teardown context manager. > > This: > > ```python > with s1 >> t1 > w1 > ``` > > does not work when w1, s1, and t1 are classic operators but if w1 is a decorated operator while s1 and t1 are classics, it works as expected. Looking into this I think the context manager is ok but think this is will be beneficial / appreciated. Sometimes context mgr will be good, sometimes it won't work with how users write dags. e.g. with this method, you could use it along with `chain`. but context mgr would require change in style. [example](https://github.com/apache/airflow/blob/main/tests/system/providers/amazon/aws/example_dms.py#L407-L429). in any case, it's easy to add and it's helpful so i don't see the harm personally. @jedcunningham ? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
