reading-am / reading

The dreams! The dreams! It's all just absurdity in the light of day but the dreams!
MIT License
10 stars 0 forks source link

Pusher sends events to all connections #365

Open sjkaliski opened 11 years ago

sjkaliski commented 11 years ago

While working on notifications @davidbyrd11 and I recognized that all comments (and now notifications) are being pushed to all currently connected users...

Pusher has a variety of channel options http://pusher.com/docs/client_api_guide/client_presence_channels

My other thought is that users subscribe to specific channels, e.g.

'notify-USERID'

We'd publish to those channels when appropriate.

thebyrd commented 11 years ago

There is also a pages.PAGEID.comments channel, but it is a little weird that every user gets notified of every comment.

@leppert Is there extra memory overhead/cost for more channels? If so, we can just filter which messages we display based on user settings. Otherwise we can create a channel for each user and push whatever info we want to nofify-USERID.

sjkaliski commented 11 years ago

@davidbyrd11 well your juggling the computation power/memory of the server vs. the client. If you have a mass volume of users messaging at a large enough volume, you're going to witness a pretty significant 1) increase in bandwidth, 2) decrease in performance on the browser.

thebyrd commented 11 years ago

@sjkaliski Let's just create notify-USERID channels then. We'll probably also have to create some type of field on the UserModel to determine what we send to that channel.

@leppert @lambjustin Have you guys gotten a chance to think about when to notify people, what options people have, is it opt-in or opt-out, etc? https://github.com/leppert/reading/issues/332

sjkaliski commented 11 years ago

@davidbyrd11 I've already started working on those channels. And yeah in the same way we have email settings you'll have to have an equivalent there as well.

leppert commented 11 years ago

Sorry guys, been out of pocket. Two things:

1 - Make sure to follow the existing naming convention for channels. They're meant to map to the restful API endpoints, accounting for the fact that Pusher doesn't allow '/' in the channel names.

2 - Pusher's recommended way of handling "feed" style channels (channels trailored to a single user) is to use their webhooks. When a user becomes active on a channel, you have Pusher publish their presence to a web hook endpoint. You then track the list of users on your end so you know which channels to publish to. For instance:

Make sense?

On Jan 6, 2013, at 1:07 AM, Steve Kaliski notifications@github.com wrote:

@davidbyrd11 I've already started working on those channels. And yeah in the same way we have email settings you'll have to have an equivalent there as well.

— Reply to this email directly or view it on GitHub.

thebyrd commented 11 years ago

I think I get it, but I'm not sure that works. What list is the ruby app adding the new user to? It seems like it would make more sense to have every user subscript to 'user.ID.notifications' and then in the comment observer we can parse the user id of the mentioned user and then Broadcast the notification to that user's channel.

sjkaliski commented 11 years ago

+1 @davidbyrd11 it makes more sense to just open these channels for subscription and shut them down on disconnect. No need to persist them... and you get to avoid the complication of managing those channels.

leppert commented 11 years ago

They recently added multi channel publishing, worth checking out: http://blog.pusher.com/multi-channel-event-publishing/

thebyrd commented 11 years ago

I'll look into it, but I don't think it works with slanger.

leppert commented 11 years ago

@davidbyrd11 ah yes, that. Forgot we're using Slanger locally.

The user.ID.notifications channel works for this use case. What I outlined above is actually more appropriate for the feed of posts from various people you follow. I either misunderstood the original question or was distracted when I wrote back.

thebyrd commented 11 years ago

The question now is how we get the user id from the client...

leppert commented 11 years ago

We could implement something like: https://graph.facebook.com/me https://dev.twitter.com/docs/api/1.1/get/account/verify_credentials

thebyrd commented 11 years ago

I'll make a reading.am/me

leppert commented 11 years ago

Go ahead and put it within the api path:

reading.am/api/me

thebyrd commented 11 years ago

k

thebyrd commented 11 years ago

I'm going to try to render the id into real_loader.coffee.erb

leppert commented 11 years ago

real_loader.coffee.erb is going to be a static file once it goes through the asset pipeline. You could remove it from the pipeline and serve it dynamically, but that means you'll have to kill the cache on it. Doesn't seem ideal.

thebyrd commented 11 years ago

good call, I forgot that cloudflare is going to cache all of the assets. On Feb 10, 2013, at 6:02 PM, Greg Leppert notifications@github.com wrote:

real_loader.coffee.erb is going to be a static file once it goes through the asset pipeline. You could remove it from the pipeline and serve it dynamically, but that means you'll have to kill the cache on it. Doesn't seem ideal.

— Reply to this email directly or view it on GitHub..