Change Request -- high risk, high reward

Jon Stanley jonstanley at gmail.com
Sun Mar 15 01:15:03 UTC 2009


I'm out, sorry for the top post. But zodbot FAS routines are not
functional due to this, so I'd be +1 here if I had a vote :)

On 3/14/09, Ricky Zhou <ricky at fedoraproject.org> wrote:
> On 2009-03-14 05:36:27 PM, Toshio Kuratomi wrote:
>> Ricky and I have both looked at the code in
>> fas/safasprovider.py::SaFasIdentity and think that it's safe to cache
>> this.  The TG-1.0.8 saprovider on which safasprovider is based does not
>> cache this but I've looked at the code and it seems like their provider
>> only uses the variable in question a maximum of two times during a
>> request.  The CSRF protection that we've enabled needs to use this
>> variable more often.
>>
>> Here's the code:
>>
>> --- a/fas/safasprovider.py
>> +++ b/fas/safasprovider.py
>> @@ -65,6 +65,7 @@ class SaFasIdentity(object):
>>
>>      def __init__(self, visit_key=None, user=None, using_ssl=False):
>>          self.visit_key = visit_key
>> +        self._visit_link = None
>>          if user:
>>              self._user = user
>>              if visit_key is not None:
>> @@ -201,9 +202,13 @@ class SaFasIdentity(object):
>>      ### TG: Same as TG-1.0.8
>>      def _get_visit_link(self):
>>          '''Get the visit link to this identity.'''
>> +        if self._visit_link:
>> +            return self.visit_link
> I already mentioned this to Toshio, but this line should be changed
> to return self._visit_link
>
>>          if self.visit_key is None:
>> -            return None
>> -        return
>> visit_class.query.filter_by(visit_key=self.visit_key).first()
>> +            self._visit_link = None
>> +        else:
>> +            self._visit_link =
>> visit_class.query.filter_by(visit_key=self.visi
>> t_key).first()
>> +        return self._visit_link
>>      visit_link = property(_get_visit_link)
>>
>> If we were outside of freeze, I would apply this as it is causing issues
>> for some of the things that talk to fas (like zodbot and developer
>> instances of pkgdb).
> +1
>
> As Toshio mentioned, we've looked at the places where this variable is
> used, and it should be safe (and easy to revert otherwise).  This will
> be a giant performance improvement for code where we call filter_private
> on a lot of users (which is a lot of places).
>
> Thanks,
> Ricky
>

-- 
Sent from my mobile device




More information about the infrastructure mailing list