Merge lp:~salgado/storm/bug-506536 into lp:storm

Proposed by Guilherme Salgado
Status: Merged
Merged at revision: not available
Proposed branch: lp:~salgado/storm/bug-506536
Merge into: lp:storm
Diff against target: 48 lines (+13/-1)
2 files modified
storm/store.py (+5/-0)
tests/store/base.py (+8/-1)
To merge this branch: bzr merge lp:~salgado/storm/bug-506536
Reviewer Review Type Date Requested Status
Gustavo Niemeyer Approve
Jamu Kakar (community) Approve
Review via email: mp+17380@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Guilherme Salgado (salgado) wrote :

This branch just changes Store.__init__() to save a reference to the
given database in self.database

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

Looks good to me.

review: Approve
lp:~salgado/storm/bug-506536 updated
355. By Guilherme Salgado

Use a getter (get_database) to access a Store's Database

Revision history for this message
Guilherme Salgado (salgado) wrote :

I should've commented here earlier as Gustavo suggested I store the
database in a private attribute and allow access to it through a getter
so that it can be overridden in subclasses, if necessary.

I've just changed my implementation as he suggested, and here's the
incremental diff

=== modified file 'storm/store.py'
--- storm/store.py 2010-01-14 14:25:18 +0000
+++ storm/store.py 2010-01-19 11:41:47 +0000
@@ -68,7 +68,7 @@
         @param database: The L{storm.database.Database} instance to use.
         @param cache: The cache to use. Defaults to a L{Cache} instance.
         """
- self.database = database
+ self._database = database
         self._event = EventSystem(self)
         self._connection = database.connect(self._event)
         self._alive = WeakValueDictionary()
@@ -81,6 +81,10 @@
         self._implicit_flush_block_count = 0
         self._sequence = 0 # Advisory ordering.

+ def get_database(self):
+ """Return this Store's Database object."""
+ return self._database
+
     @staticmethod
     def of(obj):
         """Get the Store that the object is associated with.

=== modified file 'tests/store/base.py'
--- tests/store/base.py 2010-01-14 14:25:18 +0000
+++ tests/store/base.py 2010-01-19 11:40:47 +0000
@@ -154,7 +154,7 @@
     def test_store_has_reference_to_its_database(self):
         database = DummyDatabase()
         store = Store(database)
- self.assertIdentical(store.database, database)
+ self.assertIdentical(store.get_database(), database)

 class StoreTest(object):

--
Guilherme Salgado <email address hidden>

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

Nice, makes sense.

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

Looks good indeed. Thanks!

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 2009-11-29 00:42:19 +0000
3+++ storm/store.py 2010-01-19 11:44:18 +0000
4@@ -68,6 +68,7 @@
5 @param database: The L{storm.database.Database} instance to use.
6 @param cache: The cache to use. Defaults to a L{Cache} instance.
7 """
8+ self._database = database
9 self._event = EventSystem(self)
10 self._connection = database.connect(self._event)
11 self._alive = WeakValueDictionary()
12@@ -80,6 +81,10 @@
13 self._implicit_flush_block_count = 0
14 self._sequence = 0 # Advisory ordering.
15
16+ def get_database(self):
17+ """Return this Store's Database object."""
18+ return self._database
19+
20 @staticmethod
21 def of(obj):
22 """Get the Store that the object is associated with.
23
24=== modified file 'tests/store/base.py'
25--- tests/store/base.py 2009-11-30 11:35:02 +0000
26+++ tests/store/base.py 2010-01-19 11:44:18 +0000
27@@ -149,6 +149,14 @@
28 self.assertEquals(store._cache._size, 1000)
29
30
31+class StoreDatabaseTest(TestHelper):
32+
33+ def test_store_has_reference_to_its_database(self):
34+ database = DummyDatabase()
35+ store = Store(database)
36+ self.assertIdentical(store.get_database(), database)
37+
38+
39 class StoreTest(object):
40
41 def setUp(self):
42@@ -445,7 +453,6 @@
43 self.store.flush()
44 self.assertEquals(self.store._event._hooks["flush"], set())
45
46-
47 def test_obj_info_with_deleted_object_with_get(self):
48 # Same thing, but using get rather than find.
49

Subscribers

People subscribed via source and target branches

to status/vote changes: