Code review comment for lp:~stevenk/launchpad/moar-preload-distroseries-queue

Revision history for this message
William Grant (wgrant) wrote :

143 + sprs = prefill_packageupload_caches(
144 + uploads, packageuploadsources, pubs, pucs)

Is this not already done by the underlying model method?

444 +def prefill_packageupload_caches(uploads, puses, pubs, pucs):

My eyes are burning.

Oh, god, they burn.

Can you add a bit of VWS, perhaps? There are several logical sections which could be separated.

468 + for spr in sprs:
469 + get_property_cache(spr).published_archives = []

The other three caches get away without this. Are they initialised elsewhere? Perhaps move them into this function as well.

471 + spr = get_property_cache(publication.sourcepackagerelease)

That's no SPR.

506 + @cachedproperty
507 + def published_archives(self):
508 + return list(self._published_archives())

There's no need for a separate _published_archives here, as nothing needs the actual ResultSet. Just inline it.

Additionally, you might want to invalidate this sometimes, though it's not used much so it's not hugely important. But IIRC SPPH creation is well encapsulated in PS.newSourcePublication,

review: Approve (code)

« Back to merge proposal