Skip to content

feat(taskbroker-client): expose task enqueue producer future - #741

Merged
lvthanh03 merged 1 commit into
mainfrom
tony/apply-async-with-future
Jul 6, 2026
Merged

feat(taskbroker-client): expose task enqueue producer future#741
lvthanh03 merged 1 commit into
mainfrom
tony/apply-async-with-future

Conversation

@lvthanh03

@lvthanh03 lvthanh03 commented Jul 3, 2026

Copy link
Copy Markdown
Member

Refs STREAM-1309

Modifies TaskNamespace.send_task() to return the existing producer future, and adds Task.apply_async_with_future() for dispatching a task and also returning that producer future.

This lets callers coordinate task enqueue delivery; in the case of the spans buffer enqueueing the process_segment task, it needs to know that the produce succeeded before calling done_flush_segments.

@lvthanh03
lvthanh03 requested a review from a team as a code owner July 3, 2026 16:47
@linear-code

linear-code Bot commented Jul 3, 2026

Copy link
Copy Markdown

STREAM-1309

wait_for_delivery=self.wait_for_delivery,
)

def apply_async_with_future(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why can't we make apply_async return the future? It's not a breaking change to do so

@lvthanh03 lvthanh03 Jul 6, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought there might be some risk in changing the public client API contract for every task dispatch, though I just checked in sentry and there isn't any code that uses the return value of apply_async so we won't break any exisitng logic if we make apply async return the future.

Either way, I think it's clearer to only return the producer future when the user explicitly asks for it back.

@lvthanh03
lvthanh03 merged commit 972d1d3 into main Jul 6, 2026
29 checks passed
@lvthanh03
lvthanh03 deleted the tony/apply-async-with-future branch July 6, 2026 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants