Change Request -- high risk, high reward
Ricky Zhou
ricky at fedoraproject.org
Sun Mar 15 00:50:12 UTC 2009
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/fedora-infrastructure-list/attachments/20090314/db611b8d/attachment.sig>
More information about the Fedora-infrastructure-list
mailing list