I haven't read the diff here in whole, but I'd like to note that this appears
as though it will trigger late evaluation - the display_subscribed_by property
isn't eager loaded, and a bug with a couple hundred subscriptions will cause
eager loading of a couple hundred person objects *unless* they are all
self-subscribed. It will also be a little slow because its comparing objects
not keys - the property does:
if self.person == self.subscribed_by:
return u'Self-subscribed'
else:
return u'Subscribed by %s' % self.subscribed_by.displayname
This would be a little more efficient as
if self.person_id == self.subscribed_by_id:
return u'Self-subscribed'
else:
return u'Subscribed by %s' % self.subscribed_by.displayname
but the big thing is eager loading: we need to populate the subscribing person
(but only for views that need it) en masse, rather than just-in-time.
I don't know if this will cause timeouts, but this sort of thing is one of the
key drivers for timeouts.
--
https://code.launchpad.net/~bac/launchpad/bug-799901/+merge/68590
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~bac/launchpad/bug-799901 into lp:launchpad.
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
