syedahsn commented on code in PR #38287:
URL: https://github.com/apache/airflow/pull/38287#discussion_r1538210793
##########
airflow/providers/amazon/aws/operators/neptune.py:
##########
@@ -81,17 +85,86 @@ def __init__(
self.delay = waiter_delay
self.max_attempts = waiter_max_attempts
- def execute(self, context: Context) -> dict[str, str]:
+ def execute(self, context: Context, event: dict[str, Any] | None = None,
**kwargs) -> dict[str, str]:
Review Comment:
I agree that adding the `event` dict in the `execute` method can affect the
readability of the code, but I think that its better than the alternative,
which would be to duplicate code in another method that the operator would
resume from. I think that any behavior changes should be moved to a different
method (i.e. if we want the `execute` method to do something differently
because `event` was passed), but if its just (re)loading variables that might
have changed in the Trigger, I think it should be ok to leave in the `execute`
method.
That being said, @ellisms, is there a need to reload the properties here?
The Trigger should not have changed any properties here correct?
--
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]
