Skip to content

Conversation

@mikeroll
Copy link
Contributor

@mikeroll mikeroll commented Sep 17, 2025

I don't think this needs to exist. GroupCallbacks is not in the default middlewares and has to be added and configured explicitly anyway, might as well override barrier_ttl too if desired.

Also added GroupCallbacks to the reference while I was at it.

@mikeroll mikeroll force-pushed the drop-barrier-ttl-env branch 2 times, most recently from d684aa7 to 8db4ce3 Compare September 17, 2025 12:47
@Bogdanp Bogdanp added this to the v2.0.0 milestone Oct 10, 2025
@LincolnPuzey
Copy link
Collaborator

Hi @mikeroll, is there a specific problem this is causing? If we are going to remove the env var, I think a deprecation period would be best.

@mikeroll
Copy link
Contributor Author

Not really a specific problem, I just find the current set of env vars somewhat confusing (as in, why exactly is this an env var and not a class/function/cli argument?). This is one of the more obvious cases IMO.

I would add a deprecation warning if you agree.

@LincolnPuzey
Copy link
Collaborator

LincolnPuzey commented Oct 28, 2025

Yes they are a bit ad-hoc.
I think most control worker behavior, which is not a class/function user code typically calls, so the other option for them would be a cli flag.
Given most are perhaps 'advanced customization', I don't mind them being env vars.

But also yes dramatiq_group_callback_barrier_ttl is an exception to that, where a middleware kwarg would be better.

So lets remove it, but not until 3.0.
For now, add the middleware kwarg, and deprecate the env var.
Thanks!

@LincolnPuzey LincolnPuzey removed this from the v2.0.0 milestone Oct 28, 2025
@mikeroll mikeroll force-pushed the drop-barrier-ttl-env branch from 8db4ce3 to c324a9f Compare October 28, 2025 18:22
@mikeroll mikeroll changed the title Remove the dramatiq_group_callback_barrier_ttl environment variable Deprecate the dramatiq_group_callback_barrier_ttl environment variable Oct 28, 2025
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