dstandish commented on issue #31720:
URL: https://github.com/apache/airflow/issues/31720#issuecomment-1579341940
> My question is about tracking the number of attempts. Is that inherently
something we want to avoid? The current implementation uses both a timer as
well as the number of attempts as a metric for determining how long a Trigger
should run for. Isn't that better than having just the timer?
To clarify, it's not like it's my "secret master plan" to get rid of the
tracking of attempts that you added. Let sleeping dogs lie. And it's
definitely not something I would veto anyway.
Mainly it's just that your PR called my attention to this deficiency in
logging. And we can improve it by adding a logging message.
But then perhaps in future work there won't be a need to add such logic.
Now, do we want to avoid it adding `max_attempts` to trigger? I'm sure
opinions differ on this. Personally, I don't like it because it's redundant,
potentially misleading, and just doesn't seem to me to have a reason to be
there.
Maybe if we had this better logging when you were working on that PR, you
would not have added the "max attempts" logic but instead simply counted the
attempts and printed them to log... e.g.
```
while True:
counter += 1
self.log.info("checking service; attempt %s", counter)
...
```
As discussed on the other PR, the way triggers are designed, enforcing
max_attempts is somewhat redundant and, in special cases, misleading.
Come to think of it, it's also somewhat problematic when you think about
what it means if you do some kind of exponential backoff. So, compared with
max_attempts, an absolute timeout (which is already part of the interface) is
simpler and more reliable / well-defined. So I wouldn't recommend using
max_attempts on any new operators. But of course, that's just me.
--
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]
