oh that might break polling i guess

yup :D

Checking the return value would also basically bring us back to before #4880 😅
One could argue that this PR now just moves the return True/False from before #4880 into the is_running, but IMHO it's at least still a cleaner interface. An alternative would be to make the infinite_loop variable introduced in #4880 a parameter - get_updates would set it to True, all others to False, such that the first successfull call exits. This as actually the initial draft of #4880 😆 65e6cc9

I am a bit late to those discussions :) but here goes

I agree current approach is cleaner than the old return True/False to determine retry logic in that its more explicit

However it's still way inferior to infinite_loop parameter introduced (& removed) in #4880. It would be much nicer to just pass a boolean that determines whether or not the action callback is an long living one (it can have a default value of False since polling is the only place where its True)

future me: i realize i say (long living action callback) a lot below. i mean an action callback that should be re-called even if first attempts are successful.

That would be nicer, simpler than to assume every action callback is a long living one and require the caller to write custom logic in the form of lambdas to terminate the action.

Now according to the discussion at comment #4880, i believe it was removed because it was thought redundant (infinite_loop(=True) and max_retries=(-1) meaning the same thing)
I don't think that's the case, tho the infinite_loop name might give that impression. but

• infinite_loop(=True) and max_retries=(-1): this action callback is a long living one, execute it indefinitely. e.g polling
• infinite_loop(=False) and max_retries=(-1): this action callback is not a long living one. if failed, retry it indefinitely. e.g bootstrap initialize when passing app.run_polling(bootstrap_retries=-1)

Yes it might not make sense passing infinite_loop(=True) and max_retries > 0, but i see initial commits of #4880 did handle that case. Even if left unhandled, the logic can still hold: the action call back is long living, execute it 6 times (e.g. poll for one hour with interval 10 seconds) ('')

Last note regarding #4871, i don't see an issue in incrementing retries count regardless of the success state of action_cb. Unless we want to support a use case like ('') and separate between retries because of errors, and normal operation (infinite_loop=True) retries.

Let me re-add the infinite_loop parameter, then :) Maybe a different name though - repeat_on_success? A use case for retry_on_success(=True) and max_retries > 0 (i.e. calling exactly a finite number of times) should still not be needed IMO, so I'll skip that