launchpad-dev team mailing list archive

[Danilo Šegan <danilo@xxxxxxxxxxxxx>]

У пон, 10. 08 2009. у 16:56 -0300, Christian Robottom Reis пише:
> On Mon, Aug 10, 2009 at 06:29:43PM +0200, Danilo Šegan wrote:
> > I've just noticed a problem in our person creation code:
> > 
> >   https://bugs.edge.launchpad.net/rosetta/+bug/411514
> > 
> > We are trying to create a Person record to attribute translations to,
> > however, since account for the email already exists,
> > createPersonAndEmail fails with EmailAddressAlreadyTaken exception.
> 
> Won't this happen in a wide range of situations -- essentially anywhere
> where we are attempting to create Persons lazily where there are
> existing Accounts?

It will, and we've already seen it happen with soyuz.  I can imagine it
happening with bug and code imports as well (if code imports credit
revision authors for anything by their email addresses).

> Uhm.. I think I understand -- essentially because Rosetta wants to
> credit a translation to a Person, and in this case we need to create one
> and match it with an Account that already exists. But this is a curious
> situation to me:
> 
>     - The Account itself has already been created, so it should already
>       have its own creation rationale, right?

That's right.

>     - Why does recording the Person's creation rationale matter? Or is
>       the issue more that if there is a Person record then we assume
>       that the person uses Launchpad?

It's actually the opposite (I misunderstood it myself): entire Launchpad
uses Person records to deal with any attribution.  If there is a Person
record *and* an attached Account, then that person uses Launchpad.

Person records are created for different reasons from why Accounts are
created.  I.e. Person record can be created due to:
 - account registration
 - build import
 - po file import
 - bug import
 ...

Account record could be created because of:
 - shipit registration
 - landscape registration
 - launchpad registration
 ...

The problem is that we haven't had this split from the beginning, so
many of the rationales are actually overlapping (they've been copied
from Person records to Accounts, I guess).  And what we need right now
is a Person record which has an appropriate creation rationale, and an
Account with a creation rationale which is not 'launchpad registration',
and we want that Person/Account combo not to be treated as
'uses_launchpad'.

What's more, this might even not need to be a DB field, if we can figure
out how would a transition from not using LP to using LP work, and if we
can figure out which of the existing AccountCreationRationales indicates
that a person has 'registered in LP'.

I believe the transition is the hardest bit right now, and that's where
storing a separate uses_launchpad flag seems to be the best way out.

> It seems odd that we have this situation, where parts of the Launchpad
> application are happy to work directly with Accounts and other parts
> require Persons. Is it just that OpenID and ShipIt are special in this
> sense, and that the rest of Launchpad is all engineered around Person?

I belive nothing but SSO and ShipIt is using Accounts.  Stuart can
probably figure out all the foreign references to Account easily with
his DBA voodoo (or we can use the tricks from person merging code to get
it ourselves :).

> If so, then a DB patch uses_launchpad would make sense and I'm sure we
> could get it in this cycle. But I ask myself, would there be Person
> records that had no accompanying Account records? Because that's what I
> think would be correct modeling for this situation -- nobody actually
> created an account, right?

Yes, and that's exactly what we'd get usually.  But, in this particular
case, by mere chance, that person we are trying to create only a Person
record for already has an Account (but not for Launchpad, thus Person
record doesn't exist).

That's why we don't see this bug so often: it's a relatively peculiar
set of occurences.

Cheers,
Danilo