[Planetlab-devel] inter-plc federation - email uniqueness - proposed patch & discussion

[Thierry Parmentelat]

Tony
I did some progress on this topic:
- the patch I sent does not work as is because as I suspected there are 
cases where 'peer_id' is not provided at all to validate_email
- the protected version where we filter against peer_id when it is 
provided, or against peer_id=None when it's not, is half good. With this 
version I can locally add an e-mail address that is already present from 
another peer
- but I cannot successfully cache a remote email that would have already 
locally registered. This is much more tricky because RefreshPeer turns 
out to call sync on a Person object that has no peer_id set yet.

So please do not waste too much time at this stage on 'how' to implement 
this, I'll have to spend more time on this next week.
But I'd still like your inputs on how this should be tested, because it 
clearly could have weird side-effects.


In the meanwhile, there definitely is something that you might consider 
doing on your side, in Nodes.py
 
     validate_date_created = Row.validate_timestamp
     validate_last_updated = Row.validate_timestamp
+    validate_last_contact = Row.validate_timestamp
 
     def update_last_contact(self, commit = True):

which allows the last_contact field that you recently added, to get 
correctly refreshed from the integer-date we get through GetpeerData and 
xmlrpc.

By the way, I also plan on reviewing the set of data that flows through 
GetPeerData.
It turns out this last_contact field gets cached togother with a lot 
more data that is definitely not crucial to federation, and
I am pretty sure I can substantially improve the overall performance. 
This is another story, but until I do this review you'll need this patch 
so RefreshPeer can work on your side.

-- Thierry


Thierry Parmentelat wrote:
> Hi Tony and all
>
> I am currently trying to address the following issue.
> The current status of the api & RefreshPeer thing has a major 
> drawback. During the transitional period where European users will 
> start switching from the Public PlanetLab at PLC to PlanetLabEurope, 
> there may be a need for a user to be registered at both myplc's at the 
> same time.
> In this particular case, the current software does not allow the user 
> to use the same email address on both ends. The reasons for this are that
> - AddPerson uses validate_email in Persons.py, that in turn checks for 
> any Person with that new email address and raises an exception if such 
> an entry can be found
> - RefreshPeer basically uses the same validate_email code when it 
> tries to sync newly created local objects
> -  Note however that no hard-wired constraints in the DB is enabled here
>
>
> I am currently validating the following patch in PLC/Persons.py
>         if len(domain) < 2:
>             raise invalid_email
>
> -        conflicts = Persons(self.api, [email])
> +       # Thierry - June 8 2007
> +       # check only against users on the same peer
> +       # xxx should we protect against cases where self does not hold 
> a 'peer_id' key ?
> +        conflicts = Persons(self.api, 
> {'email':email,'peer_id':self['peer_id']})
>         for person in conflicts:
>             if 'person_id' not in self or self['person_id'] != 
> person['person_id']:
>
> As you see I just alter the uniqueness check by relaxing the 
> constraint, and prevent email duplication on a given peer only.
>
> I am having a hard time setting up a reasonable test environment to 
> check this feature as thoroughly as possible, so I cannot commit on 
> its correctness yet.
> I would appreciate to know your feelings about it anyway, that might 
> save me some time especially if this is a wrong track. So:
> - first I'd like to know whether you feel comfortable with that patch 
> and hear your comments if any. In particular do you think there might 
> be cases where validate_email can be called on a 'self' that would not 
> hold a 'peer_id' filed ?
> - and second, during my test so far I've noticed that this same 
> validate_email thing was also called when performing deletions. I 
> assume this applies to all types of objects. My feeling is that this 
> sounds awkward - for instance if one changes the validation for a 
> given entity to make it more strict, then injecting the new code will 
> prevent us from cleanly deleting entities that do not fulfill the 
> validation criteria - which are precisely the ones we'd like to 
> delete. This is of course much independant from the central issue I am 
> addressing here, but your comments about this would be appreciated too.
>
>
> Thanks -- Thierry
>
> _______________________________________________
> Devel mailing list
> Devel at lists.planet-lab.org
> https://lists.planet-lab.org/mailman/listinfo/devel