potiuk commented on issue #18721: URL: https://github.com/apache/airflow/issues/18721#issuecomment-934374354
I have not thought that through entirely but I think that can have undesireable consequences. The "has_succes/failure_callbcks" property is especially added to handle "Serialized DAG" case - rather than serializing/deserializing the calbacks, only the "has" properties are serialized. So those properties are needed. And also I cannot immeediately see why the callbacks could not be set afterwards - but still during parsing of the DAG. On the other hand, it would be interesting to hear @vilozio what is WHEN you would like to set the callbacks. If you would like to set them during "execute" methods of some of the tasks, then it's almost for sure not going to happen due to security and performance reasons. People might be tempted to do so though, if we allow to override it in general. The fact that you add it at initialization only is "stronger" than "during parsing" and makes it pretty obvious you should not do it "after" the DAG has been parsed and serialized. I am just wondering if there is a reason why you would like to be able to set it during DAG parsing and still not be able to pass it in the __init__ method? Because I would actually be tempted to turn the callbacks into read-only properties after initialization to avoid any temptation of trying to set it during initialization. @kaxil @ashb -> do I get it right? Any other concerns/comments? -- 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]
