Skip to content
This repository was archived by the owner on Oct 29, 2020. It is now read-only.

More Northstar sync fixes#6411

Merged
DFurnes merged 12 commits intoDoSomethingArchive:devfrom
DFurnes:northstar-migrate-touchups
May 10, 2016
Merged

More Northstar sync fixes#6411
DFurnes merged 12 commits intoDoSomethingArchive:devfrom
DFurnes:northstar-migrate-touchups

Conversation

@DFurnes
Copy link
Contributor

@DFurnes DFurnes commented May 6, 2016

What's this PR do?

This PR fixes a number of smaller issues with Northstar migration and syncing. The highlights:

  • Fixes issue where 1234445555@mobile "placeholder" emails were being sent with mobile users and causing validation errors on Phoenix. These are now ignored when forming the payload. (cc3640b)
  • When forwarding new registrations, set phoenix as the signup source. This was previously only happening for batch migrated users. (0096db3)
  • I also did a little refactoring to extract the build_ns_user call out of the methods which make the HTTP requests to Northstar (c946d7b). This allows us to easily chain another request to register a user if the "update" request failed without having to try to re-build the whole user object. (9492787)
  • Outputs progress to the console when migrating now! (d100755)

I'd recommend reviewing commit-by-commit since otherwise some of the changes get jumbled.

How should this be manually tested?

I've tested by manually checking that users are created/updated against my local Northstar, and by adding dpm calls to trace which hooks/methods get called.

Any background context you want to provide?

I'm not 100% sure that triggering a "register" whenever an "update" call fails will work... I feel like there might be some sort of conflict that's preventing those users from being registered in the first place. But it'll help surface that issue if so, and if not it's a nice way to "stop the bleeding" while continuing to investigate to figure out the underlying issue. 🔍

What are the relevant tickets?

References #6406 and #6260.


For review: @angaither

DFurnes added 12 commits May 2, 2016 15:13
This was previously loading user out of order (perhaps because some of
the low UIDs were added manually?) which made it look like people were
getting skipped when skimming logs.
We encode JSON in this script, and then again in the function so this
is not necessary!
This allows us to make the call without needing a full form array to
rebuild the query (for example, so we can chain a “register” call after
an “update” call).
We’ve had some “update” calls result in 404s, so try to create those
users when that occurs.
Some users have a last name set on their profile. If we have it, we
should send that to Northstar!
@angaither
Copy link
Contributor

👍

@DFurnes DFurnes merged commit ac14278 into DoSomethingArchive:dev May 10, 2016
@DFurnes DFurnes deleted the northstar-migrate-touchups branch May 10, 2016 17:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants