launchpad-dev team mailing list archive

[Celso Providelo <celso.providelo@xxxxxxxxxxxxx>]

On Wed, Aug 12, 2009 at 9:11 AM, Guilherme Salgado<salgado@xxxxxxxxxxxxx> wrote:
> On Tue, 2009-08-11 at 17:01 -0400, Stuart Bishop wrote:
>> > On Tue, Aug 11, 2009 at 12:57:28PM -0300, Guilherme Salgado wrote:
>> >> Another (simpler) option is to abuse the fact that the Person and
>> >> Account tables are not linked by foreign keys and create the Person
>> >> entry when needed but not link it to the existing Account. That way,
>> >> everything would just magically work as the Person would seem to have no
>> >> account. The only thing we'd have to do is change the registration/login
>> >> (maybe reset password too) process to link the Person to the Account
>> >> when they are not linked. Of course, by doing that we would also be
>> >> hiding any bugs that might cause associated Person/Account entries to
>> >> not be linked when they should.
>>
>> Please don't abuse the data model :-P If you do that, the alerts that
>> have been setup to detect this data corruption will go off. There
>> should be foreign key constraints to ensure that all these links
>> remain sane, and it is unfortunate that we cannot use them due to our
>> replication requirements.
>>
>> The way we designed this situation to be handled is you create the
>> person record and link it to the existing emailaddress and account if
>> they exist.
>>
>> If we need to differentiate Person records created through these
>> automatic approaches (imports from external bug trackers, pofile
>> imports, branch imports, mailing list imports) from Person records
>> being actively used, we should add flags. We should not extend the
>> meaning of creation_rationale - new flags and fields is better. I
>> certainly don't want to turn it into a bitmap field that makes it
>> impossible to query sanely.
>
> Agreed.
>
>>
>> I guess the main reason we want to differentiate users who have
>> actively created a Launchpad Person from the import created Person is
>> so we don't spam people who haven't signed up themselves (which would
>> happen if someone subscribed them to a bug. I think an 'acivated' or
>> 'deactivated' boolean would handle this - do we need something more
>> complex than this?
>
> In fact, what we should aim for is to make sure is to have these Person
> entries show up as placeholder profiles in the web UI and not include
> them in the ValidPerson* vocabs, so that they don't get linked to
> anything -- that's much better than trying to fix all places that may
> send email to people to skip the ones which have uses_lp=False.
>
>>
>> We should certainly create the Person records with a specific
>> creation_rationale.
>>
>> We should certainly set the hide-emailaddress flag.
>
> Agreed on both.

Right, guys, I had a call with Salgado this morning and presented my
changes to IPersonSet.ensurePerson() allowing it to create missing
Person for existing Accounts.

http://pastebin.ubuntu.com/264993/

The only remaining problem is that the Person being created in this
circumstance would be presented as an active user in LP, which is not
the case for any callsite of ensurePerson.

I have a feeling that we all agreed on adding a new flag to Person
('active' possibly) which would be set as False in line 25 of the diff
above. At that point we can also set hide_emailaddress True.

Assuming that we do have an agreement about it, would it be possible
to accelerate the process and add the new flag in the production DB
ASAP so we can CP the ensurePerson changes at least in iron.c.c and
unblock debian imports appropriately ?

Otherwise I will have to add a hack in gina to ignore imports in this
condition for reestablishing the service until we find a better
solution.

Any other ideas ?
-- 
Celso Providelo <celso.providelo@xxxxxxxxxxxxx>
IRC: cprov,  Jabber: cprov@xxxxxxxxxx, Skype: cprovidelo
1024D/681B6469 C858 2652 1A6E F6A6 037B  B3F7 9FF2 583E 681B 6469