Skip to content
This repository has been archived by the owner on Dec 5, 2022. It is now read-only.

Send 'session start' and 'io start' push pings #1340

Merged
merged 3 commits into from
May 11, 2015
Merged

Conversation

x1ddos
Copy link
Contributor

@x1ddos x1ddos commented May 10, 2015

Can't verify with dev/stage server until @jeffposnick implements #1248 but confirmed by tests.
Closes #1293.

@x1ddos
Copy link
Contributor Author

x1ddos commented May 10, 2015

@ebidel I realised phase3 is due May 26 which is actually the latest this PR should be deployed on prod. so, probably no need for merge-into-phase2.

@ebidel
Copy link
Contributor

ebidel commented May 10, 2015

Yep. Strictly phase 3.

On Sun, May 10, 2015, 3:33 PM alex notifications@github.com wrote:

@ebidel https://github.com/ebidel I relized phase3 is due May 26 which
is actually the latest this PR should be deployed on prod. so, probably no
need for merge-into-phase2.


Reply to this email directly or view it on GitHub
#1340 (comment)
.

@@ -8,3 +8,6 @@ cron:
- description: sync schedule data
url: $PREFIX$/sync/gcs
schedule: every 30 minutes
- description: sessions start notifications
url: $PREFIX$/task/clock
schedule: every 1 min
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is every 1min excessive? Should we put the cron on 5min? GCM might take longer than that though to propagate.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see below you need this to run more frequently.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I wanted to give GCM as much time as possible to deliver msgs.

@x1ddos
Copy link
Contributor Author

x1ddos commented May 11, 2015

PTAL

@jeffposnick
Copy link
Contributor

This looks good to me. I'm working on the client-side changes now, but I'm not anticipating that you'd have to change anything you're doing server-side.

x1ddos added a commit that referenced this pull request May 11, 2015
Send 'session start' and 'io start' push pings
@x1ddos x1ddos merged commit 98611a6 into master May 11, 2015
@x1ddos x1ddos deleted the push-start-backend branch May 11, 2015 19:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Send "update: start" push notifications from the backend
3 participants