Code review comment for lp:~milo/linaro-patchmetrics/user-migration

Revision history for this message
Milo Casagrande (milo) wrote :

On Wed, Oct 23, 2013 at 4:36 PM, Paul Sokolovsky
<email address hidden> wrote:
> Review: Approve
>
> One thing is that I thought this would be belong to django custom management commands, but I see that patchmetrics has that patterns of bin/ utils using _pythonpath module ok.

I personally do not like it either, but I do not like either the code
base to be honest.

> But the strategy looks pretty surprising. I always thought of something like: when we have both old (LP) and new (Crowd) user (so user would need to login via Crowd), and we migrate all objects from one user id to another. so, we retain both users in there (as backout strategy for example).
>
> So, approach of this script with "destructive" migration of user's identity from Launchpad to Crowd feels almost like cheating.

You are right, it is indeed like cheating.

But the more I looked at the DB though, the more I thought that this
was the most likely and easiest solution to migrate in just one batch.
If we ended up adding new users, as I was trying to do in the
beginning, we would have had to mass-update all the teams in
patchemtrics, and the user-teams membership (and still I was not sure
if we could have guaranteed the correct results).

> Well, if it works, I cannot really say something bad about it, because it's indeed nice automated en-masse solution. One thing I'd mention though is matter of how complete this "identity switching" is. For example, there's likely openid data linked to this account. I don't think this would interfere with something, but well, worth mentioning.

I tried to clean up also those, we will not need them anymore, but it
will take a little bit of cleaning because there are external keys
involved. We can do that, probably not as programmatically as this MP,
but it can be done with some SQL queries.

As far as we are concerned, the openid data does not pose a problem
for patchmetrics to work.
For how complete it is, you should be able to get everything you had
with the old authentication method.

--
Milo Casagrande | Automation Engineer
Linaro.org <www.linaro.org> │ Open source software for ARM SoCs

« Back to merge proposal