-
Notifications
You must be signed in to change notification settings - Fork 160
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
Add support for using Registry's via tuple #239
Conversation
a55a5b1
to
c5158f0
Compare
Great work! I have added some minor comments. We also need to add tests for Broadway.get_rate_limiting, Broadway.test_messages, Broadway.producer_names, and Broadway.topology. So I think we should move the test that we have so far into its own describe block, so we can add these new tests. WDYT? |
Sounds good @josevalim ! Updates coming, thanks so much for your quick feedback |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright @josevalim see updates!
Assuming we're getting close, let me know where in docs I can make updates
@acco fantastic! I have dropped the last batch of comments. Then please rebase and we should be good to go! |
Support overriding process_name/2 in Broadway module so that all processes in topology can be started with a via tuple.
Thanks @josevalim, great notes! Just incorporated, rebased, and pushed. Assuming latest looks good, I can squash and we can merge. Let me know if it needs any other tweaks |
@acco I added some very minor comments and @whatyouhide suggested a guard, which can be very helpful, and we can ship it! |
Great feedback/tweaks @josevalim @whatyouhide , thank you! Pushed latest. Happy to squash before merge, lmk |
💚 💙 💜 💛 ❤️ |
Wooo! Thanks for all your help 🙏 |
It seems like this callback was accidentally generated inside the module calling "use Broadway" instead of being part of the Broadway behaviour. This callback was originally introduced in #239.
It seems like this callback was accidentally generated inside the module calling "use Broadway" instead of being part of the Broadway behaviour. This callback was originally introduced in #239.
Support overriding process_name/2 in Broadway module so that all processes in topology can be started with a via tuple.
Discussed in #236