Merge lp:~mwhudson/launchpad/permit_timeout_from_features-on-participation-bug-861510 into lp:launchpad
Status: | Merged |
---|---|
Approved by: | Michael Hudson-Doyle |
Approved revision: | no longer in the source branch. |
Merged at revision: | 14143 |
Proposed branch: | lp:~mwhudson/launchpad/permit_timeout_from_features-on-participation-bug-861510 |
Merge into: | lp:launchpad |
Diff against target: |
344 lines (+84/-38) 11 files modified
lib/canonical/launchpad/scripts/__init__.py (+5/-5) lib/canonical/launchpad/webapp/adapter.py (+5/-3) lib/canonical/launchpad/webapp/authorization.py (+16/-1) lib/canonical/launchpad/webapp/ftests/test_adapter.py (+0/-16) lib/canonical/launchpad/webapp/interaction.py (+22/-1) lib/canonical/launchpad/webapp/interfaces.py (+19/-0) lib/canonical/launchpad/webapp/servers.py (+4/-2) lib/canonical/launchpad/webapp/tests/test_servers.py (+3/-1) lib/canonical/testing/layers.py (+9/-6) lib/lp/code/tests/test_doc.py (+1/-1) lib/lp/testing/__init__.py (+0/-2) |
To merge this branch: | bzr merge lp:~mwhudson/launchpad/permit_timeout_from_features-on-participation-bug-861510 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Benji York (community) | code | Approve | |
Martin Pool (community) | Approve | ||
Review via email: mp+78355@code.launchpad.net |
Commit message
[r=benji,mbp][bug=623199,861510] Move permit_
Description of the change
Hi,
This branch does two things:
1) It creates a pattern for storing stuff in the participation (aka request in an appserver context) rather than thread local variables, and
2) Uses this new pattern to store the flag indicating whether to read the hard_timeout feature flag in this way rather on a thread local.
Part 2) fixes bug 861510, which could have been fixed in other ways, but I've been thinking about a way to do 1) and start fixing bug 623199. I have two and a half failed attempts to store the feature controller itself (only) on the participation, but they all got a bit confusing and I decided to pick an easier target (I wanted to fix _some_ problem in this way, to validate the technique). I quite like the fix -- because the flag defaults to False, I can kill off the half baked attempts we have to clear this flag when it needs to be cleared and can be confident that it can't escape the test isolation (this is one of the reasons I just landed a branch that unconditionally tears down the interaction between tests <wink>).
The core understanding I reached is that in Launchpad, the participation is only ever:
(a) an instance of canonical.
(b) an instance of canonical.
(c) some subclass of canonical.
So we only need to add an attribute to instances of these three classes and then we can confidently access it whenever there is a zope interaction set up (whenever we're "logged in" in some sense, although this includes the anonymous "user").
In the thread linked to from bug 623199, I proposed using the annotations dictionary that Launchpad's browser requests already have, but when push came to shove I didn't really see the point and just stuck another attribute on -- if we were to use annotations, we'd have to come up with a constant containing the key and import that around and it just felt unnecessary (also some page templates read the features attribute off the request already, so supporting them was easier this way in my previous branches).
I've successfully tested the branch in EC2.
Cheers,
mwh
I'm no kind of authority on what is tasteful in Zope or Launchpad, but this does sound good to me.
I think you should fix your 'XXX explain more' ;-) by adding something similar to the text in <https:/ /bugs.launchpad .net/launchpad/ +bug/861510/ comments/ 3>, otherwise people will ask, as I did, why you need special handling of the hard_timeout feature flag, rather than just looking it up in the usual way.