Merge lp:~lifeless/storm/bug-620615 into lp:storm

Proposed by Robert Collins on 2010-08-19
Status: Rejected
Rejected by: Gustavo Niemeyer on 2010-08-19
Proposed branch: lp:~lifeless/storm/bug-620615
Merge into: lp:storm
Diff against target: 12 lines (+1/-1)
1 file modified
storm/store.py (+1/-1)
To merge this branch: bzr merge lp:~lifeless/storm/bug-620615
Reviewer Review Type Date Requested Status
Gustavo Niemeyer 2010-08-19 Disapprove on 2010-08-19
Review via email: mp+33144@code.launchpad.net

Description of the change

Disable the 'please contact the developers' code path, as it seems to happen with a pretty normal query (see bug 620615 for details including how to reproduce).

I'd love to know the risks around this; its one component blocking a fairly significant fix for the milestones page https://edge.launchpad.net/landscape/+milestone/later :)

Thanks in advance for any feedback you can give me about how to test this: I would just delete the whole code block by preference.

My theory is that the cache changes done recently make this a standard (if uncommon) situation, rather than an impossible one.

To post a comment you must log in.
Gustavo Niemeyer (niemeyer) wrote :

As we discussed online, there's an actual bug here. The runtime error is protecting from data corruption. With Robert's help, we've figured what was wrong, and I'll produce a branch with actual tests soonish.

review: Disapprove

Unmerged revisions

373. By Robert Collins on 2010-08-19

Disable an untested code path that makes things break.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'storm/store.py'
2--- storm/store.py 2010-08-19 07:43:34 +0000
3+++ storm/store.py 2010-08-19 19:30:55 +0000
4@@ -755,7 +755,7 @@
5 if variable.is_defined():
6 continue
7 lazy_value = variable.get_lazy()
8- if not (lazy_value is None or lazy_value is AutoReload):
9+ if False and not (lazy_value is None or lazy_value is AutoReload):
10 # This should *never* happen, because whenever we get
11 # to this point it should be after a flush() which
12 # updated the database with lazy values and then replaced

Subscribers

People subscribed via source and target branches

to status/vote changes: