Skip to content

fix(taskbroker): Replace worker_map Default Instead of Merging#756

Merged
george-sentry merged 4 commits into
mainfrom
george/push-taskbroker/no-worker-map-default
Jul 16, 2026
Merged

fix(taskbroker): Replace worker_map Default Instead of Merging#756
george-sentry merged 4 commits into
mainfrom
george/push-taskbroker/no-worker-map-default

Conversation

@george-sentry

@george-sentry george-sentry commented Jul 16, 2026

Copy link
Copy Markdown
Member

For some reason, the Seer push taskbroker was getting a connection refused error when trying to reach the taskworker service. After some deep investigation, I found that Figment is very silly. Instead of replacing worker_map with whatever the user provides, it merges it.

So if you provide TASKBROKER_WORKER_MAP={seer=foo}, its value will be {sentry=http://127.0.0.1:50052, seer=foo} rather than just {seer=foo}. I strongly suspect that is the cause.

I have changed the code to replace worker_map with whatever the user provides, falling back on the previous default if the user doesn't provide anything.

@george-sentry
george-sentry requested a review from a team as a code owner July 16, 2026 18:28
Comment thread src/main.rs
Comment thread src/main.rs

for (application, endpoint) in config.worker_map.clone() {
let worker = match Worker::connect(config.clone(), endpoint).await {
let worker = match Worker::connect(config.clone(), endpoint.clone()).await {

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.

This is a good point, can we have the startup fail if the worker map is empty?

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.

Yepp, I added a validate attribute to the worker_map field to make sure it has at least one entry. Also, I changed the code to use a default when the user doesn't provide anything. But this time, the user's worker_map replaces the default instead of merging with it.

@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 779da43. Configure here.

Comment thread src/config/mod.rs
@george-sentry george-sentry changed the title fix(taskbroker): Remove worker_map Default fix(taskbroker): Replace worker_map Default Instead of Merging Jul 16, 2026
@george-sentry
george-sentry merged commit 24a05bf into main Jul 16, 2026
29 checks passed
@george-sentry
george-sentry deleted the george/push-taskbroker/no-worker-map-default branch July 16, 2026 19:02
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.

3 participants