dstandish commented on code in PR #32099:
URL: https://github.com/apache/airflow/pull/32099#discussion_r1242759002
##########
airflow/models/baseoperator.py:
##########
@@ -1001,6 +1001,17 @@ def __enter__(self):
def __exit__(self, exc_type, exc_val, exc_tb):
SetupTeardownContext.set_work_task_roots_and_leaves()
+ @staticmethod
+ def add_task_to_context(task):
Review Comment:
OK looks good.
Just want to suggest one last rename... and I'm trying to channel my inner
@uranusjr here.... I think that maybe we should call it (the private one)
BaseSetupTeardownContext instead of AbstractSetupTeardownContext because, I
think Abstract strongly implies that it is not meant to be used directly, but
only via subclasses. Whereas, the word Base seems to better tolerate direct
usage -- and we use it directly. That sound right to you? I know historically
it's common to use Base for abstract classes to, but it just seems like the
better of the two for something that is used as a concrete class. Maybe you
have a better idea...
Since it's gonna be a private class I guess it doesn't matter too much but
it does feel like we should use something other than abstract.
--
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]
