Bug 1358123: Add handler for FxA deleted queue #35

pmac · 2017-08-14T14:27:25Z

No description provided.

pmac · 2017-08-14T17:03:43Z

@rfk This expects the message from the queue to be simply

{
  "event": "delete",
  "uid": "<the fxa id>"
}

But from the look of the code you pasted in the bug that structure might be embedded deeper under a Message key. Do I need to alter what I have? We'll find out in testing of course.

rfk · 2017-08-15T02:46:20Z

Yeah, unfortunately due to the way that SQS works, I think you'll get a top-level "Message" key with the actual JSON payload in it as a string.

pmac · 2017-08-15T13:45:06Z

Interesting. I don't get that from other SQS queues. Perhaps it's a consequence of the libraries we're using to access SQS? Not a big deal either way, just want to get it right :) Thanks!

pmac · 2017-09-07T18:01:53Z

basket/news/tasks.py

+            'email': email,
+            'lang': lang,
+            'newsletters': settings.FXA_REGISTER_NEWSLETTER,
+            'source_url': settings.FXA_REGISTER_SOURCE_URL,


@rfk I think this should work well. Do you know if you're currently sending the source_url based on the actual FxA domain that sent the event? I'm not sure it matters much for us, but I could do something fancy with this based on whether we're consuming the dev, stage, or prod queue.

It's a config value that we could change for each environment, but I think right now it just defaults to "accounts.firefox.com" even in dev. Let me know if you have a use for us changing it and I'll file a bug about updating the config.

pmac · 2017-09-07T18:03:18Z

basket/news/tasks.py

+        })
+
+
+@et_task
 def add_fxa_activity(data):


@rfk after the other things, I believe the only other HTTP requests you send us are for this function which records account activity and includes the FxA ID, the user_agent, and a first_device boolean. Is this another event that will be on the queue?

Yes, there will be a "login" event in the queue which we currently handle here:

https://github.com/mozilla/fxa-basket-proxy/blob/master/lib/events/index.js#L86

Our handling of this event does two things. It registers the activity by posting to /fxa-activity/:

https://github.com/mozilla/fxa-basket-proxy/blob/master/lib/events/index.js#L114

And it also does a bit of magic where, if the user logged in with certain special values of the "utm_campaign" query parameter, it adds them to special subscriptions in basket:

https://github.com/mozilla/fxa-basket-proxy/blob/master/lib/events/index.js#L94

It's a lightly hacky way for us to tunnel metrics info through FxA for various first-run-page experiments.

If you're happy to take over handling of those events as well, that'll be great! We'll have to be careful though because IIUC the /fxa-activity/ part is not idempotent.

after the other things, I believe the only other HTTP requests you send us
are for this function which records account activity

FWIW we also provide a "communication preferences" screen in FxA settings, where the user can change their choice of the marketing opt-in checkbox that they made during signup. We currently action this by making calls to the existing basket web API, but we could consider whether there's a better way.

FWIW we also provide a "communication preferences" screen in FxA settings, where the user can change their choice of the marketing opt-in checkbox that they made during signup. We currently action this by making calls to the existing basket web API, but we could consider whether there's a better way.

That user interaction seems like a fine place for using the basket api, and if you're happy to keep doing it that's fine with me. I think we can move the activity handling to the queue, but that's a good point about it being different and not idempotent... Perhaps we should hold of on that bit and do it in a separate bug and sometime later?

Filed #37 to cover this BTW.

glogiotatidis

A question about msg.delete(). Looks good!

glogiotatidis · 2017-09-11T12:16:10Z

basket/news/management/commands/process_fxa_queue.py

+                        continue
+
+                    statsd.incr('fxa.events.message.success')
+                    msg.delete()


Would it be useful to delete the msg just before et_task completes to ensure that we don't miss any msgs in case salesforge is down for example?

I was thinking we could rely on celery retries for those cases, but it is possible to lose some if it's down for a long time. I'm not currently passing the message object to the task (not sure how well it'd go on the celery queue), but I could probably figure out how to delete the message from the task instead of the consumer; I'm just not sure it's worth going to that trouble. What do you think?

The other option is to just not use celery tasks for this and to just record the messages as they come in from the queue consumer process. I didn't do that because celery gives us retries and much more scalability, but it could work better in the case of message deletion timing.

I hear you, I don't know if that's worth the trouble either.

Actually re-reading this made me think: Do you plan to run this command periodically? If yes how will it get triggered?

No, it just runs indefinitely via that infinite loop. Same as the process_donations_quqeue command.

pmac · 2017-09-14T14:32:31Z

basket/news/tasks.py

+
+    _update_fxa_info(email, lang, fxa_id)
+
+    if subscribe:


@rfk I guess we'll need to implement this bit as well, and add those new campaigns from the bug?

https://github.com/mozilla/fxa-basket-proxy/blob/13ed0f5206f9150566135d3a0973ac358cf1c0f7/lib/events/index.js#L96

Actually nevermind. Won't need to do that until we takeover the "login" events in #37. I'll add a note there to be sure to do this bit.

pmac · 2017-09-14T15:31:42Z

@rfk @jbuck I think we're ready to go with this. Let's coordinate on a time when we can:

Get messages flowing to the stage and prod queues
Deploy basket changes
Stop posting to the /fxa-register/ basket endpoint

jbuck · 2017-09-14T15:39:36Z

@pmac do we need to do these all around the same period of time, or can I get the messages flowing to stage & prod queues before the basket & fxa deploys?

pmac · 2017-09-14T16:50:49Z

It'd be nice if it were "around" the same time just to reduce the number of duplicate queue messages and POSTs coming from FxA, and to minimize the queue size at basket deploy. Is there something in FxA to deploy? The stopping of POSTing to basket for these things I guess?

But to be clear they don't have to be at exactly the same time, but the closer the better.

jbuck · 2017-09-14T17:09:39Z

It looks like there's a "Purge Queue" action I can do, so I can deploy the queues to stage & prod now and purge before we deploy the FxA changes.

pmac · 2017-09-14T17:16:08Z

It looks like there's a "Purge Queue" action I can do, so I can deploy the queues to stage & prod now and purge before we deploy the FxA changes.

Works for me. Thanks! 👍

pmac · 2017-09-14T17:17:08Z

Just to be clear, this code will go to dev and stage when I merge to master. I'll tag and push to the prod branch when we're ready for production.

Also subscribe users that opt-in to newsletters

rfk · 2017-09-17T22:04:10Z

Stop posting to the /fxa-register/ basket endpoint

This should be pretty trivial, we can include this change in FxA train-96, going to stage this week and deploying next week.

rfk · 2017-09-17T22:04:50Z

So we could aim to give live week of 25th September on both sides?

pmac · 2017-09-18T01:24:10Z

Sounds good to me. Thanks!

rfk · 2017-09-25T22:48:31Z

So we could aim to give live week of 25th September on both sides?

FYI this is in stage right now on our side

pmac · 2017-09-26T02:21:49Z

Great. Deploying to stage here as well then.

pmac · 2017-09-26T02:50:56Z

These changes have been deployed to basket dev and stage and the queue processor containers have been started. Let's get some more test data flowing :)

rfk · 2017-09-26T02:54:52Z

I just signed up as "ryan-loves-newsletters@rfk.id.au" and checked the opt-in box, do you see any events from me?

pmac · 2017-09-26T03:16:06Z

Actually I'm having trouble with the stage queue :(

botocore.exceptions.ClientError: An error occurred (AWS.SimpleQueueService.NonExistentQueue) when calling the ReceiveMessage operation: The specified queue does not exist or you do not have access to it.

I get that with the prod and stage settings. The dev queue works. Not sure why yet.

rfk · 2017-09-26T03:18:29Z

/cc @jbuck ^

pmac · 2017-09-26T03:22:59Z

I've had a look around the IAM roles for access to these queues and it looks right to me. Perhaps @jbuck and I can troubleshoot tomorrow.

jbuck · 2017-09-27T16:04:07Z

I've fixed this in stage - it'll be going out with prod later today

pmac · 2017-09-27T16:33:32Z

Confirmed the stage queue is now working! Or at least it's allowing me to connect (haven't verified any actual messages yet). Prod queue is still broken but it sounds like you're on it. Thanks @jbuck ! 🥇

pmac force-pushed the add-fxa-queue-1358123 branch from c106f13 to 87c9bb9 Compare August 14, 2017 17:06

pmac force-pushed the add-fxa-queue-1358123 branch from 87c9bb9 to 70d29e4 Compare August 15, 2017 13:48

pmac force-pushed the add-fxa-queue-1358123 branch from 56bf5d6 to 358a0b5 Compare September 6, 2017 20:04

pmac commented Sep 7, 2017

View reviewed changes

pmac force-pushed the add-fxa-queue-1358123 branch from 358a0b5 to 4b56d7c Compare September 8, 2017 15:52

pmac mentioned this pull request Sep 8, 2017

Move FxA Activity data from a view to the queue #37

Closed

pmac requested a review from glogiotatidis September 8, 2017 20:05

glogiotatidis approved these changes Sep 11, 2017

View reviewed changes

pmac commented Sep 14, 2017

View reviewed changes

pmac added 2 commits September 14, 2017 13:24

Bug 1358123: Add handler for FxA deleted queue

4c94485

Move handling of FxA info from view to queue

bb93707

Also subscribe users that opt-in to newsletters

pmac force-pushed the add-fxa-queue-1358123 branch from 4b56d7c to bb93707 Compare September 14, 2017 17:31

rfk mentioned this pull request Sep 18, 2017

Stop forwarding "verified" events to /fxa-register/ on basket mozilla/fxa-basket-proxy#43

Closed

pmac merged commit 6751a96 into mozmeao:master Sep 26, 2017

pmac deleted the add-fxa-queue-1358123 branch September 26, 2017 02:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug 1358123: Add handler for FxA deleted queue #35

Bug 1358123: Add handler for FxA deleted queue #35

pmac commented Aug 14, 2017

pmac commented Aug 14, 2017

rfk commented Aug 15, 2017

pmac commented Aug 15, 2017

pmac Sep 7, 2017

rfk Sep 7, 2017

pmac Sep 7, 2017

rfk Sep 7, 2017

pmac Sep 8, 2017

pmac Sep 8, 2017

glogiotatidis left a comment

glogiotatidis Sep 11, 2017

pmac Sep 11, 2017

glogiotatidis Sep 13, 2017

pmac Sep 13, 2017

pmac Sep 14, 2017

pmac Sep 14, 2017

pmac commented Sep 14, 2017

jbuck commented Sep 14, 2017

pmac commented Sep 14, 2017

jbuck commented Sep 14, 2017

pmac commented Sep 14, 2017

pmac commented Sep 14, 2017

rfk commented Sep 17, 2017

rfk commented Sep 17, 2017

pmac commented Sep 18, 2017

rfk commented Sep 25, 2017

pmac commented Sep 26, 2017

pmac commented Sep 26, 2017

rfk commented Sep 26, 2017

pmac commented Sep 26, 2017

rfk commented Sep 26, 2017

pmac commented Sep 26, 2017

jbuck commented Sep 27, 2017

pmac commented Sep 27, 2017

Bug 1358123: Add handler for FxA deleted queue #35

Bug 1358123: Add handler for FxA deleted queue #35

Conversation

pmac commented Aug 14, 2017

pmac commented Aug 14, 2017

rfk commented Aug 15, 2017

pmac commented Aug 15, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glogiotatidis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pmac commented Sep 14, 2017

jbuck commented Sep 14, 2017

pmac commented Sep 14, 2017

jbuck commented Sep 14, 2017

pmac commented Sep 14, 2017

pmac commented Sep 14, 2017

rfk commented Sep 17, 2017

rfk commented Sep 17, 2017

pmac commented Sep 18, 2017

rfk commented Sep 25, 2017

pmac commented Sep 26, 2017

pmac commented Sep 26, 2017

rfk commented Sep 26, 2017

pmac commented Sep 26, 2017

rfk commented Sep 26, 2017

pmac commented Sep 26, 2017

jbuck commented Sep 27, 2017

pmac commented Sep 27, 2017