Skip to content

feat(cli): Add worker CLI with configurable options#760

Merged
evanh merged 4 commits into
mainfrom
evanh/feat/cli-command
Jul 23, 2026
Merged

feat(cli): Add worker CLI with configurable options#760
evanh merged 4 commits into
mainfrom
evanh/feat/cli-command

Conversation

@evanh

@evanh evanh commented Jul 21, 2026

Copy link
Copy Markdown
Member

Summary

Adds a taskbroker_client CLI for running workers, with a click option for every constructor argument of TaskWorker and PushTaskWorker.

  • worker — runs a pull-mode TaskWorker, with an option for each TaskWorker.__init__ argument.
  • push_worker — runs a push-mode PushTaskWorker, with an option for each PushTaskWorker.__init__ argument.
  • spawn / scheduler — existing example helpers.

New tunables

  • --grpc-max-message-size (on push_worker): threaded through a new grpc_max_message_size argument on PushTaskWorker. When unset, the worker falls back to the TASKBROKER_GRPC_MAX_MESSAGE_SIZE env var, so existing env-based deployment config keeps working.
  • --track-producer-futures/--no-track-producer-futures (on both commands): arroyo only reads ARROYO_TRACK_PRODUCER_FUTURES from the environment, so the command sets that env var from the flag before starting the worker. It's on both commands because the future-tracking producer runs in child processes for both pull and push modes.

Test plan

  • black, isort, flake8, and mypy pass via pre-commit.
  • Verified both commands build and expose the new options, and that the arroyo flag round-trips through the env var arroyo reads (True → "True" enables, False → "False" disables).

🤖 Generated with Claude Code

Add a taskbroker_client CLI exposing `worker` (pull) and `push_worker` (push) commands, each with a click option for every TaskWorker / PushTaskWorker constructor argument, plus `spawn` and `scheduler` helpers.

Add `--grpc-max-message-size` to `push_worker`, threaded through PushTaskWorker as a new `grpc_max_message_size` argument that falls back to the TASKBROKER_GRPC_MAX_MESSAGE_SIZE env var when unset.

Add `--track-producer-futures/--no-track-producer-futures` to both worker commands, which sets the ARROYO_TRACK_PRODUCER_FUTURES env var arroyo reads to toggle producer future tracking.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@evanh
evanh requested a review from a team as a code owner July 21, 2026 19:55
Comment thread clients/python/src/taskbroker_client/cli.py
Comment thread clients/python/src/taskbroker_client/cli.py
Comment thread clients/python/src/taskbroker_client/cli.py
Comment thread clients/python/src/taskbroker_client/cli.py
Comment thread clients/python/src/taskbroker_client/cli.py

@untitaker untitaker left a comment

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.

oh wait. can we have one cli for push/non-push? otherwise i think we might have a hard time unifying the templates too (each app will need to configure where the push cli is mounted, and where the non-push cli is mounted)

or a click group for push/non-push CLIs, and apps attack the click group to their main group

@evanh

evanh commented Jul 22, 2026

Copy link
Copy Markdown
Member Author

or a click group for push/non-push CLIs, and apps attack the click group to their main group

What do you mean by this? Isn't that what they can already do with the worker / push_worker commands?

@untitaker

untitaker commented Jul 22, 2026

Copy link
Copy Markdown
Member

What do you mean by this? Isn't that what they can already do with the worker / push_worker commands?

right now there are two entrypoints for taskworker CLI that can be mounted in the application's CLI in different places. it is permitted to do this, though a bad idea:

sentry run worker
sentry push-worker

it would be more restrictive, and less boilerplate in applcation code if instead only one CLI needs to be mounted.

sentry run worker
sentry run push-worker

could be:

sentry run worker --push-mode

or:

sentry run worker push
sentry run worker pull

in the ops repo template, one then only has to configure sentry run worker as "the prefix where the worker CLI lives", and everything else can be done using shared templates

@evanh

evanh commented Jul 22, 2026

Copy link
Copy Markdown
Member Author

@untitaker I think I see your point, but I deliberately separated them because they require different arguments. If I had one command worker, that command would have to take the union between all the different CLI args, and it would be unclear which ones were needed for which mode. What do you think?

@untitaker

untitaker commented Jul 22, 2026

Copy link
Copy Markdown
Member

I deliberately separated them because they require different arguments

yeah i see that. but you can make them sub-commands of a @click.group (in this repo), which then would be added to another click group in the app (sentry run worker pull and sentry run worker push)

Combine the standalone `worker` and `push_worker` commands into a single `worker` click group with `pull` and `push` subcommands, so it composes into the app's command tree (e.g. `sentry run worker pull` / `worker push`).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit bf572dd. Configure here.

) -> None:
from taskbroker_client.worker import TaskWorker

os.environ["ARROYO_TRACK_PRODUCER_FUTURES"] = str(track_producer_futures)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Track futures flag overwrites env

Medium Severity

Both pull and push always assign ARROYO_TRACK_PRODUCER_FUTURES from the Click flag, whose default is false. Deployments that enable tracking only via the environment lose that setting when the new worker commands run without --track-producer-futures, unlike --grpc-max-message-size, which leaves the env var in effect when unset.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit bf572dd. Configure here.

@untitaker untitaker left a comment

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.

i think spawn and scheduler can be in that group too, i forgot to mention them but they are also distinct entrypoints that need separate wiring up rn

Comment on lines +210 to +211
exitcode = worker.start()
raise SystemExit(exitcode)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The pull command worker incorrectly re-raises KeyboardInterrupt on graceful shutdown, causing a non-zero exit code instead of a clean exit with code 0.
Severity: HIGH

Suggested Fix

Modify TaskWorker.start() to catch the KeyboardInterrupt and allow the function to complete and return an exit code, similar to how PushTaskWorker.start() is implemented. This will prevent the exception from propagating and ensure the CLI can call SystemExit(0) for a clean shutdown.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: clients/python/src/taskbroker_client/cli.py#L210-L211

Potential issue: When the `pull` command worker receives a `SIGINT` or `SIGTERM` signal
for a graceful shutdown, the underlying `TaskWorker.start()` method re-raises a
`KeyboardInterrupt` after handling the signal. This unhandled exception propagates up,
preventing the CLI from executing `raise SystemExit(exitcode)`. Instead, the process
terminates with a non-zero exit code (e.g., 130), incorrectly signaling a crash to
orchestration systems like Kubernetes. The `push` command worker correctly swallows this
exception and exits cleanly with code 0.

@evanh
evanh merged commit 9067c69 into main Jul 23, 2026
29 of 30 checks passed
@evanh
evanh deleted the evanh/feat/cli-command branch July 23, 2026 16:30
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.

2 participants