Merge lp:~jkakar/storm/block-access-context-manager into lp:storm

Proposed by Jamu Kakar
Status: Merged
Approved by: Thomas Herve
Approved revision: 370
Merged at revision: 546
Proposed branch: lp:~jkakar/storm/block-access-context-manager
Merge into: lp:storm
Diff against target: 91 lines (+79/-0)
2 files modified
storm/store.py (+18/-0)
tests/store/block.py (+61/-0)
To merge this branch: bzr merge lp:~jkakar/storm/block-access-context-manager
Reviewer Review Type Date Requested Status
Colin Watson Approve
James Henstridge Approve
Stuart Bishop (community) Approve
Review via email: mp+32551@code.launchpad.net

Description of the change

This branch introduces the following changes:

- A new block_access context manager blocks database access for one
  or more stores in the managed scope.

To post a comment you must log in.
Revision history for this message
Stuart Bishop (stub) wrote :

The years in the copyright statement in tests/store/block.py are old. Otherwise all good.

review: Approve
Revision history for this message
James Henstridge (jamesh) wrote :

It looks like this test will only pass on Python >= 2.6 (since it uses the with statement without a __future__ pragma). Is that desirable?

I don't think there is as much need to keep Python 2.4 compatibility as there once was (Zope 3 and Launchpad have been updated since then), but are we at a point where we want to drop 2.5 support too?

Revision history for this message
Jamu Kakar (jkakar) wrote :

James:

Yeah, I wondered about that when I implemented the change. I put it
in its own module to keep Storm workable with older versions of
Python. Perhaps we can do two things to make it better:

1. Use a __future__ import to make it usable with Python 2.5.

2. Add a version check in the tests to make sure the test suite
   continues to run on Python 2.4.

What do you think?

Revision history for this message
James Henstridge (jamesh) wrote :

Adding the __future__ import seems worth doing.

I'm not sure whether 2.4 compatibility is that important now though. We mainly kept it for support for apps stuck using versions of Zope 3 from before that framework was updated to work with current versions of Python (namely Launchpad). Given that Zope 3 has moved on, it isn't clear whether we need to maintain 2.4 compatibility now.

In contrast, the previous Ubuntu LTS release (Hardy) didn't have 2.6, so it is quite possible that we've still got some boxes that still don't have 2.6 rolled out yet. I assume it'd be similar for some users on other enterprise distros.

It would be nice to require 2.6 at some point though, since it would let us start preparing for Python 3 support, but I'm not convinced we're there yet.

Anyway: with a __future__ import, I've got no objection to this change.

review: Approve
Revision history for this message
Jamu Kakar (jkakar) wrote :

James:

Awesome, thanks, I'll add the __future__ import before merging.
Regarding Python 2.4 my feeling is that we should announce
deprecation when we release 0.18 and then officially drop support
when we release 0.19. That'll give people some heads up about the
change.

Revision history for this message
Colin Watson (cjwatson) wrote :

I'm writing this from the increasingly strange future of 2020, where it's very clear we don't need to preserve 2.4 compatibility any more, so I'm going to go ahead and merge this without that (and with just a minor adjustment to cope with tests.* having been moved to storm.tests.*).

review: Approve

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-05 19:43:41 +0000
3+++ storm/store.py 2010-08-13 02:57:43 +0000
4@@ -1805,3 +1805,21 @@
5 pass
6
7 AutoReload = AutoReload()
8+
9+
10+class block_access(object):
11+ """
12+ Context manager blocks database access by one or more L{Store}s in the
13+ managed scope.
14+ """
15+
16+ def __init__(self, *args):
17+ self.stores = args
18+
19+ def __enter__(self):
20+ for store in self.stores:
21+ store.block_access()
22+
23+ def __exit__(self, exc_type, exc_val, exc_tb):
24+ for store in self.stores:
25+ store.unblock_access()
26
27=== added file 'tests/store/block.py'
28--- tests/store/block.py 1970-01-01 00:00:00 +0000
29+++ tests/store/block.py 2010-08-13 02:57:43 +0000
30@@ -0,0 +1,61 @@
31+# Copyright (c) 2006, 2007 Canonical
32+#
33+# Written by Gustavo Niemeyer <gustavo@niemeyer.net>
34+#
35+# This file is part of Storm Object Relational Mapper.
36+#
37+# Storm is free software; you can redistribute it and/or modify
38+# it under the terms of the GNU Lesser General Public License as
39+# published by the Free Software Foundation; either version 2.1 of
40+# the License, or (at your option) any later version.
41+#
42+# Storm is distributed in the hope that it will be useful,
43+# but WITHOUT ANY WARRANTY; without even the implied warranty of
44+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
45+# GNU Lesser General Public License for more details.
46+#
47+# You should have received a copy of the GNU Lesser General Public License
48+# along with this program. If not, see <http://www.gnu.org/licenses/>.
49+
50+from storm.databases.sqlite import SQLite
51+from storm.exceptions import ConnectionBlockedError
52+from storm.store import Store, block_access
53+from storm.uri import URI
54+
55+from tests.helper import TestHelper, MakePath
56+
57+
58+class BlockAccessTest(TestHelper):
59+ """Tests for L{block_access}."""
60+
61+ helpers = [MakePath]
62+
63+ def setUp(self):
64+ super(BlockAccessTest, self).setUp()
65+ database = SQLite(URI("sqlite:"))
66+ self.store = Store(database)
67+
68+ def test_block_access(self):
69+ """
70+ The L{block_access} context manager blocks access to a L{Store}. A
71+ L{ConnectionBlockedError} exception is raised if an attempt to access
72+ the underlying database is made while a store is blocked.
73+ """
74+ with block_access(self.store):
75+ self.assertRaises(ConnectionBlockedError, self.store.execute,
76+ "SELECT 1")
77+ result = self.store.execute("SELECT 1")
78+ self.assertEqual([(1,)], list(result))
79+
80+ def test_block_access_with_multiple_stores(self):
81+ """
82+ If multiple L{Store}s are passed to L{block_access} they will all be
83+ blocked until the managed context is left.
84+ """
85+ database = SQLite(URI("sqlite:%s" % self.make_path()))
86+ store = Store(database)
87+ with block_access(self.store, store):
88+ self.assertRaises(ConnectionBlockedError, self.store.execute,
89+ "SELECT 1")
90+ self.assertRaises(ConnectionBlockedError, store.execute,
91+ "SELECT 1")

Subscribers

People subscribed via source and target branches

to status/vote changes: