Add support for using Registry's via tuple #239
Conversation
acco
force-pushed
the
master
branch
2 times, most recently
from
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.
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