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.  
   
   Thinking again... 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]

Reply via email to