-
Notifications
You must be signed in to change notification settings - Fork 72
Re-using or pooling connections with different notifications to avoid spamming the APN services #31
Comments
Yeeeaah, well... I spent a couple hours looking at ways to implement this. I couldn't really find any library providing a SSLConnectionPool class without having to pull in a bunch of unnecessary dependencies (e.g. geventhttpclient). I guess we could probably copy and adapt some code from geventhttpclient or urllib3's HTTPConnectionPool (both are used under the MIT license, which is very liberal), but it would take some time to do this properly. I won't have time to do this for now. In the meantime, I'll try to whip up some not-really-reusable-but-quick fix on my end, but I don't think I'll want to open a pull request for that.. |
Thanks @mbargiel. As you pointed out, I'd like to take a look at connection pooling/reusing. Thanks for pointing me to |
@stephenmuss I think it would be worth sharing my last attempt with you. I'll try to find a moment to open a pull request this weekend or the next. |
@mbargiel sounds great! |
At first sight, it looks like the network logic should be decoupled from the APNService model and be executed by a Celery worker. That would solve many issues, including the requirement of a persistent connection. |
You're right in that. As a matter of fact, we had some ideas to refactor However, we probably don't want to tie this library to specific frameworks. I'm not sure what you mean about "the requirement of a persistent Le vendredi 22 novembre 2013, dperetti a écrit :
|
If the network logic is refactored, I'm sure it will be easy to support Celery without requiring it. |
It definitely should be. I see the optional integration of Celery in the Stay tuned! :) Le vendredi 22 novembre 2013, dperetti a écrit :
|
Is there any progress in this? I definitely agree that it's worth decoupling the networking from the model itself. |
Not on my end, even if I brought it up in the first place. sheepish I If you are interested, I could probably share the prototype code for the Le lundi 16 février 2015, kalvish21 notifications@github.com a écrit :
|
We ran into a problem recently. Unless I'm mistaken, the typical usage of the APNService.push_notification_to_devices method seems to be oriented towards sending push notifications occasionally to a large number of devices.
However we're using this app to implement a real-time messaging system in our app, which means we're very very often sending notifications to select devices. Therefore, the current implementation constantly opens and closes SSL connections, which could be interpreted by the APN service as a DoS. Indeed, https://developer.apple.com/library/ios/technotes/tn2265/_index.html states:
"As is documented in the Local and Push Notification Programming Guide, developers are expected to open a connection and leave it open. If a connection is opened and closed repeatedly, APNs will treat it as a denial of service attack and block connections for a period of time."
I think a great feature (or even, a necessary feature, for this type of use case) would be to keep a connection cached in the model for each APNService instance ID, perhaps with a TTL to close if after a certain amount of time has passed without sending notifications, so that they can be re-used. Extra care would be necessary for the async/multi-threaded (or multi-process?) scenarios, such as when using Django celery.
What do you think?
(Note: I'll do some investigation on my end and see if I can implement something that works.)
The text was updated successfully, but these errors were encountered: