Code review comment for lp:~allenap/maas/shared-to-per-tenant-storage

Revision history for this message
Gavin Panella (allenap) wrote :

> [2]
>
> +# -*- coding: utf-8 -*-
>
> I don't see why this is required…?
> (fwiw, the only file with that are the auto-generated migration files)
>
> PS: I suspect this is just because of → … s/→/->/ ?

Yes, that's exactly why. I'll remove it for the benefit of
encoding-challenged text editors :)

>
> [3]
>
> 76 + try:
> 77 + legacy_user = User.objects.get(username=legacy_user_name)
>
> If a migrations runs and we are in a position to run that code
> (i.e. more than one user, no 'provider-state' file), if it happens
> that there is a *real* user with that username, it won't do the
> right thing. I reckon this is pretty improbable but maybe we should
> at least raise and exception in that case. What do you think?

I want the code to be safe to run multiple times, so I think it's
important to be able to pick up an existing legacy user. However, I'll
make the legacy user similar to the other system users, in that it
won't have a UserProfile. Then I can check that here and raise an
exception if it appears to be a regular user with the same name.

Sound okay?

I

« Back to merge proposal