875945 - Change login server DB backend to MySQL with live migration, change UUID from email to a number

Status: RESOLVED FIXED

(Webmaker Graveyard :: Login, defect)

Description

[Kieran Sedgwick (:sedge)]

There are two things that must be reviewed with regards to the Login server's persistent data storage:

1) Should we be using MongoDB?
2) Should we be using the user's email as their UUID in the database we choose?

Currently, we ARE using MongoDB, and we ARE using a Webmaker's email as their UUID.  There are going to be implications to changing either or both of these, and this bug should clear those up.

Comment 1

[Jon Buckley [:jbuck]]

1) Nah, we're not really storing data like that
2) No, just an auto-incrementing ID

Comment 2

[David Humphrey (:humph)]

To sum up:

1) Let's get the login tests landed (bug 873091)
2) Let's move to MySQL (needs new bug)
3) Let's drop ._id and use an auto increment field (this can happen in 2) above)

Next question: In webmaker-loginapi we use an "ID" for getUser and isAdmin, see https://github.com/humphd/node-webmaker-loginapi/blob/master/index.js#L41.  There are going to be some cascading implications for the move to an integer from an email here, since many clients are going to have the email via Persona routes of one kind or another.  Once 1-3 are done above, we need to carefully trace the use of `id` through *.webmaker.org.

CC'ing some folks who should be aware of this.

Comment 3

[Mike Beltzner [:beltzner, not reading bugmail]]

Did this get accomplished? I think it did, but want to be sure before just closing it out.

Comment 4

[Jon Buckley [:jbuck]]

It didn't get done and I don't know if attempting to do this with 2 days until we hit the ON button is a good idea.

But maybe it is, since it means we won't need to migrate any data from mongodb to mysql...

Comment 5

[Kieran Sedgwick (:sedge)]

This patch includes switching the primary key from a user's email to a generated UUID.

Comment 6

[Kieran Sedgwick (:sedge)]

Attachment: https://github.com/mozilla/login.webmaker.org/pull/127

Comment 7

[Kieran Sedgwick (:sedge)]

Comment on attachment 772845 [details] [review]
https://github.com/mozilla/login.webmaker.org/pull/127

Almost there.  Almost.

Comment 8

[Kieran Sedgwick (:sedge)]

Comment on attachment 772845 [details] [review]
https://github.com/mozilla/login.webmaker.org/pull/127

Woo!

Comment 9

[Matthew Schranz [:mjschranz]]

Comment on attachment 772845 [details] [review]
https://github.com/mozilla/login.webmaker.org/pull/127

Looking good, but see the pull request for all of my comments.

Comment 10

[Matthew Schranz [:mjschranz]]

Comment on attachment 772845 [details] [review]
https://github.com/mozilla/login.webmaker.org/pull/127

See PR.

Comment 11

[Chris DeCairos (:cade)]

Comment on attachment 772845 [details] [review]
https://github.com/mozilla/login.webmaker.org/pull/127

This looks great, left a bunch of comments in the PR. awesome stuff!

Comment 12

[Kieran Sedgwick (:sedge)]

Comment on attachment 772845 [details] [review]
https://github.com/mozilla/login.webmaker.org/pull/127

Bugs ahoy

Comment 13

[David Humphrey (:humph)]

Comment on attachment 772845 [details] [review]
https://github.com/mozilla/login.webmaker.org/pull/127

A bunch of comments/questions in the PR.  I should probably look at this again when it's been through a round of fixes.

Comment 14

[Kieran Sedgwick (:sedge)]

Comment on attachment 772845 [details] [review]
https://github.com/mozilla/login.webmaker.org/pull/127

MOAH EYEZ

Comment 15

[Kieran Sedgwick (:sedge)]

Comment on attachment 772845 [details] [review]
https://github.com/mozilla/login.webmaker.org/pull/127

1 more major revision, then flagging you again.

Comment 16

[Chris DeCairos (:cade)]

Comment on attachment 772845 [details] [review]
https://github.com/mozilla/login.webmaker.org/pull/127

Looking great, will review again when you push up your next set of fixes and moar testz.

Comment 17

[Chris DeCairos (:cade)]

Comment on attachment 772845 [details] [review]
https://github.com/mozilla/login.webmaker.org/pull/127

ARRRRRRR+

Comment 18

[Kieran Sedgwick (:sedge)]

Comment on attachment 772845 [details] [review]
https://github.com/mozilla/login.webmaker.org/pull/127

Slowly but surely...

Comment 19

[Jon Buckley [:jbuck]]

Comment on attachment 772845 [details] [review]
https://github.com/mozilla/login.webmaker.org/pull/127

Unassigning from myself while Kieran does some code changes

Comment 20

[Kieran Sedgwick (:sedge)]

Comment on attachment 772845 [details] [review]
https://github.com/mozilla/login.webmaker.org/pull/127

They were good times.  I laughed, I cried, I - well, I mostly cried.

Lets get 'er done!

Comment 21

[Kieran Sedgwick (:sedge)]

Comment on attachment 772845 [details] [review]
https://github.com/mozilla/login.webmaker.org/pull/127

Switching to Matt since Jon is on PTO.

Comment 22

[Kieran Sedgwick (:sedge)]

Comment on attachment 772845 [details] [review]
https://github.com/mozilla/login.webmaker.org/pull/127

Added Jon back to review for testing towards deployment on production.  I'm leaving Matt on for code review.

Comment 23

[Matthew Schranz [:mjschranz]]

Comment on attachment 772845 [details] [review]
https://github.com/mozilla/login.webmaker.org/pull/127

Jon, can you help me understand why you want the code base to basically carry two systems? Why force someone to keep mongo and mysql running? Why are we changing how we have always done things (migration scripts) to keeping around extra code that really isn't needed?

Personally, I'm against it and don't understand why this decision was made. Can you help me here?

Comment 24

[Jon Buckley [:jbuck]]

Comment on attachment 772845 [details] [review]
https://github.com/mozilla/login.webmaker.org/pull/127

If support for mongodb is completely removed, there's going to be downtime while we migrate user accounts from mongodb to mysql. And this wouldn't just be downtime for some portions of the site, it'd be for all of *.webmaker.org & *.makes.org.

It's more of a technical burden for us to carry, but I don't expect us to carry it for long. Once it's been landed, we can write a migration script to move the webmaker accounts that haven't been migrated yet. Once they've all been migrated, then we can have a follow-up patch to remove the mongodb bits left in login.webmaker.org.

Comment 25

[Kieran Sedgwick (:sedge)]

mjschranz, jbuck - To make it less of a development burden, I can make mongoDB an optional dev dependancy using an environment variable (or procfile).

Comment 26

[Kieran Sedgwick (:sedge)]

This patch being landed is now more dependant on the (lengthy) review process than it is on my attention to the bug, so I'm moving on to other work. 

As reviews happen, which I assume they will as soon as there's time for them, I'll come back to this.

Comment 27

[Jon Buckley [:jbuck]]

Found a bug here, if fullName is null then the SQL insert fails. Checking on prod shows:

> db.users.count({fullName: null})
631

I'll need to fix this up before we ship this to staging

Comment 28

[Jon Buckley [:jbuck]]

Comment on attachment 772845 [details] [review]
https://github.com/mozilla/login.webmaker.org/pull/127

r+ with the deployment problem noted above. I'll need to fix that, then this can be merged to staging

Comment 29

[[github robot]]

Commits pushed to master at https://github.com/mozilla/login.webmaker.org

https://github.com/mozilla/login.webmaker.org/commit/01941d69ff719464dd4ddab8ffed52907474002e
[bug 875945] Changed over from MongoDB to MySQL

https://github.com/mozilla/login.webmaker.org/commit/e5c327cc44b1acd1adb437e178e2b0ae31882e7b
Merge pull request #127 from ksedge/bug875945

[bug 875945] Changed over from MongoDB to MySQL

Comment 30

[Kieran Sedgwick (:sedge)]

This patch is up and running on staging - I'm keeping this bug open until we have it running properly on production.

Comment 31

[Kieran Sedgwick (:sedge)]

Running on prod! Heck yeah!