У пон, 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 _______________________________________________ Mailing list: https://launchpad.net/~launchpad-dev Post to : [email protected] Unsubscribe : https://launchpad.net/~launchpad-dev More help : https://help.launchpad.net/ListHelp
