ferruzzi commented on PR #66141:
URL: https://github.com/apache/airflow/pull/66141#issuecomment-5460140806
@wjddn279 Sorry this sat so long, and thanks for the work here.
Since this PR was opened, #71192 tackled the same issue from the other
direction. It adds PATCH /{callback_id}/run for the single-use callback token,
along with the token datamodel, the security plumbing and the
`executors/workloads/callback.py` side. That supersedes the `/run` half of
this PR, and it's approved and mergeable now, so I don't think it makes sense
to carry both implementations of that endpoint.
What that PR does not have is your PATCH `/{callback_id}/state`, which is
still useful and I'd like to keep it. Without it, a running callback has no
Execution API path to report terminal state back, and that feels like a useful
part of the callback execution API umbrella in #62887.
Rather than closing this outright, would you be willing to rebase onto
#71192 and cut it down to just the state-lifecycle endpoint? That's a much
smaller PR than the 948 lines here, it stops competing over
`routes/callbacks.py`, and it lands the part that Sean didn't cover. Two
things from Ash's earlier review still apply to whatever ships: the TIToken
rename now that the token serves callbacks too, and the docstring that surfaces
in the generated OpenAPI spec.
If you'd rather not pick it back up, that's completely fine, just say so and
I'll close this and open a follow-up with just that part once #71192 merges. I
can cherry-pick your commit so the authorship stays with you. Or maybe that
results in co-author, I'm not entirely sure, but either way you'll be credited.
--
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]
