Merge lp://staging/~fgallina/rnr-server/oauth-username-generation-cleanup into lp://staging/rnr-server
Proposed by
Fabián Ezequiel Gallina
| Status: | Rejected |
|---|---|
| Rejected by: | Fabián Ezequiel Gallina |
| Proposed branch: | lp://staging/~fgallina/rnr-server/oauth-username-generation-cleanup |
| Merge into: | lp://staging/rnr-server |
| Prerequisite: | lp://staging/~fgallina/rnr-server/remove-reviewer-limit-for-clicks |
| Diff against target: |
41 lines (+3/-17) 1 file modified
src/core/utilities.py (+3/-17) |
| To merge this branch: | bzr merge lp://staging/~fgallina/rnr-server/oauth-username-generation-cleanup |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Matias Bordese (community) | Approve | ||
|
Review via email:
|
|||
Commit message
Do not try to generate usernames, usernames are the unique consumer_key
To post a comment you must log in.
Unmerged revisions
- 299. By Fabián Ezequiel Gallina
-
Do not try to generate unique usernames, usernames are the unique consumer_key
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
LGTM
(13:00:56) fgallina: matiasb: no, before the SSO v2 migration, the v1 api account data would return a username to be used and we didn't make any adjustments for it: http://
(13:03:41) matiasb: fgallina: right, but I think that username that was returned before wasn't the consumer_key, so users created at that time could have a username different from the consumer_key, and in rare cases, match a consumer_key for a user we are creating now... makes sense?
(13:05:30) fgallina: matiasb: yeah, but I think the chances that we really have a consumer_key that conflicts with some existing username is really unlikely to be worried about.
(13:05:59) fgallina: matiasb: moreover I planned to migrate a fair good set (people that reviewed click packages) to start holding the consumer_key as username.
(13:06:12) matiasb: ah, that was my next question (re migration)
(13:06:32) matiasb: why not migrating them all?
(13:07:54) fgallina: matiasb: yeah, we can do that. I was worried about the amount of users we may have to migrate, but I don't see another issue why we can't do that.
(13:08:47) fgallina: matiasb: also, I planned this to be done not as a data migration but as a manual thing to be run in servers, so it doesn't depend on the deployment and we don't add more things to the migration list.
(13:09:21) matiasb: ack