seanghaeli commented on code in PR #76571:
URL: https://github.com/apache/airflow/pull/76571#discussion_r4496116799
##########
task-sdk/src/airflow/sdk/api/client.py:
##########
@@ -1181,8 +1184,34 @@ def __init__(self, client: Client):
self.client = client
def run(self, callback_id: uuid.UUID) -> None:
- """Exchange the single-use callback token for an execution token."""
- self.client.patch(f"callbacks/{callback_id}/run")
+ """
+ Exchange the single-use callback token for an execution token.
+
+ The server uses up the token once it gets the request, so only retry
when the request never
+ reached it, and give a slow server longer to answer.
+ """
+ try:
Review Comment:
Having the server hand out a token again would mean that anyone holding a
copy of the callback token (it sits in the broker message) gets a new working
execution token. That would defeat the purpose of #71192, and the callback
token would no longer strictly be single-use.
With this client fix, the callback token remains strictly single-use, all
that changes is that the client is careful about when it retries. The client
knows which requests never made it to the server, and only retries those. The
server only sees the ones that made it, so it can't tell a retry from someone
replaying a copy.
For the lost response and the 502/503 during restarts, the client can't know
if the token was used, so it fails the callback rather than retry. I can
provide a follow-up that dispatches the callback again with a fresh token in
these situations, but it would have a bigger review surface and might not make
sense to backport to 3.4.1, which is why I'll treat it as a follow-up.
--
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]
