launchpad-reviewers team mailing list archive

["j.c.sackett" <jonathan.sackett@xxxxxxxxxxxxx>]

The proposal to merge lp:~jcsackett/launchpad/i-hate-storm-base-storm-705197 into lp:launchpad has been updated.

Description changed to:

Summary
=======

Several models uses storm.base.Storm as their parent class. These classes end up introducing cache errors as they don't have the necessary storm hooks to clear/invalidate caches (as other classes e.g. SQLBase do).

This branch introduces a new class, descended from storm.base.Storm, that adds the same storm hook functionality as in SQLBase.

Preimplementation
=================

Spoke with Robert Collins, Curtis Hovey, and William Grant about what exactly was needed.

Proposed Fix
============

Create a new class, StormBase, that descends from storm.base.Storm and adds the hooks needed (borrowed from SQLBase).

Implementation
==============

lib/lp/service/database/stormbase.py
------------------------------------

Adds the new StormBase class.

All other files
---------------

Updates uses of storm.base.Storm to lp.services.database.StormBase

Tests
=====

Basically you have to run ec2. You can try:

bin/test -m lp.bugs
bin/test -m lp.codehosting
bin/test -m lp.registry

But that's not guaranteed to test all areas changed. I've had to ec2 test to track down any breaks.

Demo & QA
=========

Not terribly qa-able. Make sure bugtracking and bugtasks work--they were heaviest effected. But honestly, if this weren't working, lp.dev will just crash all over the place.

Lint
====

= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:  

lib/canonical/launchpad/database/librarian.py
lib/lp/bugs/model/apportjob.py  
lib/lp/bugs/model/bugjob.py  
lib/lp/bugs/model/bugsubscription.py  
lib/lp/bugs/model/bugsubscriptionfilter.py  lib/lp/bugs/model/bugsubscriptionfilterimportance.py  lib/lp/bugs/model/bugsubscriptionfilterstatus.py  lib/lp/bugs/model/bugsubscriptionfiltertag.py  
lib/lp/bugs/model/bugtracker.py  
lib/lp/bugs/model/bugwatch.py  
lib/lp/code/model/branchmergeproposaljob.py  
lib/lp/registry/model/nameblacklist.py  
lib/lp/registry/model/persontransferjob.py  
lib/lp/services/database/stormbase.py  
lib/lp/soyuz/model/archivejob.py  
lib/lp/soyuz/model/distributionjob.py

./lib/lp/bugs/model/apportjob.py
      35: 'ITemporaryStorageManager' imported but unused
     163: E222 multiple spaces after operator
     210: E202 whitespace before ')'
./lib/lp/bugs/model/bugjob.py
      26: 'removeSecurityProxy' imported but unused
     142: E222 multiple spaces after operator./lib/lp/bugs/model/bugsubscriptionfiltertag.py
      18: E302 expected 2 blank lines, found 1
./lib/lp/code/model/branchmergeproposaljob.py
       5: E303 too many blank lines (3)
     289: E202 whitespace before ')'
./lib/lp/registry/model/persontransferjob.py
      14: 'SQLObjectNotFound' imported but unused
./lib/lp/soyuz/model/archivejob.py
     136: E222 multiple spaces after operator

I'm going to fix lint after review; the diff has a lot of little one-off lines as it is, and I didn't want to clutter it up.



For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/i-hate-storm-base-storm-705197/+merge/47269
-- 
https://code.launchpad.net/~jcsackett/launchpad/i-hate-storm-base-storm-705197/+merge/47269
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/i-hate-storm-base-storm-705197 into lp:launchpad.