Skip to content

Bugfix | Fix the webhook connection and queues. #429

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Kyon147
Copy link
Owner

@Kyon147 Kyon147 commented Jun 9, 2025

Issue:

At the moment we return null if connection and queue are not set in the config which by default that is how it is.

This means that we always force webhooks onto the default queue so that using onQueue or onConnectionare always null from the dispatch.

… returning null means it overrides any on job connections and quees meaninf it alway defaults.
@Kyon147
Copy link
Owner Author

Kyon147 commented Jul 16, 2025

@enmaboya @rvibit can you please review my code if you have time. No worries if not, but I'd rather not self approve my own code.

@rvibit
Copy link
Contributor

rvibit commented Jul 18, 2025

@enmaboya @rvibit can you please review my code if you have time. No worries if not, but I'd rather not self approve my own code.

looks fine to me, it'll use default connection and queue when not provided explicitly correct?

@Kyon147
Copy link
Owner Author

Kyon147 commented Jul 18, 2025

@rvibit yeah - the issue with null, is that it is causing everything to end up on the default queue, which means that you can't set a queue inside a job for example or of the back of another job.

As the main handler is always taking precedence, so omitting the method when not needed, means it just allows for it to be overwritten downstream as expected.

If left blank, then it will just fallback to the default queue as you mentioned.

@Kyon147 Kyon147 requested a review from rvibit July 18, 2025 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants