-
Notifications
You must be signed in to change notification settings - Fork 41
Feature/improve batch messaging #30
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
Feature/improve batch messaging #30
Conversation
Instead of always using the applications' eventloopgroup's next loop use the client's eventloop instead. This should lessen the need to hop eventloops.
The `validate_only` parameter did not seem necessary and since it was causing a warning I opted to remove it.
MihaelIsaev
left a comment
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.
@siemensikkema overall new batch send works well? I just can't test it at the moment.
|
We'll test it more on a project tomorrow. So far it seemed to work well for single notifications sent using the batch method. |
Pretty sure. The documentation mentions a maximum of 500 and this python library also has a limit of 500. |
|
But earlier in the same document they mention
But that must be legacy since they also support 500 messages in Python but only mentions Java and Node.js. |
@siemensikkema could you please confirm that you tested it for multiple notifications sent using the batch method and I'll merge and release it. Thank you 🙏 |
|
@siemensikkema have you had a chance to test it on sending a message to more than one recipient? 🙂 |
|
@MihaelIsaev Hi! Thanks for following up! Yes, we've been using this in production for over a week now and there are no complaints from the client anymore |
|
It is now available since |
Implements #29