Code review comment for lp:~huwshimi/launchpad/avatars-everywhere-712894

Revision history for this message
Robert Collins (lifeless) wrote :

On Mon, Nov 7, 2011 at 6:42 PM, Huw Wilkins <email address hidden> wrote:
> Thanks Rob, I'll add the feature flag. I was kind of hoping I would get away with the change as custom_logo_url and custom_icon_url are very similar, but I don't understand the underlying code. I'll push some changes.

So here is what happens:
Our Person object has a link to a LibraryFileAlias for each of the icon sizes.
The link has to be traversed to render a URL to the librarian for the
icon/logo/branding.
So currently, where we have had performance issues and setup eager
loading, we traverse the *icon* link for all the Person objects at
once, which means one pretty efficient DB query. The results for this
then sit in the storm cache until the template renders them.

Your change makes the template rendering look at 'logo' not 'icon',
and that won't be in the storm cache, so a page with (say) 100
subscribers, will suddenly do 100 extra DB queries.

-Rob

« Back to merge proposal