Merge lp:~robru/friends/csv-model-schema into lp:friends
- csv-model-schema
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Ken VanDine |
Approved revision: | 185 |
Merged at revision: | 181 |
Proposed branch: | lp:~robru/friends/csv-model-schema |
Merge into: | lp:friends |
Diff against target: |
383 lines (+113/-93) 10 files modified
data/model-schema.csv (+23/-0) debian/friends.install (+1/-0) friends/tests/mocks.py (+9/-2) friends/tests/test_protocols.py (+5/-5) friends/utils/base.py (+13/-13) friends/utils/model.py (+39/-44) service/src/service.vala (+14/-26) setup.py (+3/-1) tools/debug_live.py (+3/-0) tools/debug_slave.py (+3/-2) |
To merge this branch: | bzr merge lp:~robru/friends/csv-model-schema |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Ken VanDine | Approve | ||
PS Jenkins bot (community) | continuous-integration | Approve | |
Review via email: mp+157246@code.launchpad.net |
Commit message
Unify model schema definition into a CSV.
Description of the change
The holy grail ;-)
Define the model schema in one unified location, and parse it in from both model.py and service.vala. This way schema changes are much less error-prone.
PS Jenkins bot (ps-jenkins) wrote : | # |
Ken VanDine (ken-vandine) wrote : | # |
You should try to read from the source tree before trying to read the installed file.
Ken VanDine (ken-vandine) wrote : | # |
Whoops, forgot to set it to needs fixing.
- 180. By Robert Bruce Park
-
Stop parsing CSV at import-time.
This commit makes it a lot easier to force the reading of the schema
from the source tree before installing Friends.
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:180
http://
Executed test runs:
SUCCESS: http://
Click here to trigger a rebuild:
http://
- 181. By Robert Bruce Park
-
Unify some import logic.
- 182. By Robert Bruce Park
-
Make a comment more explicit.
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:181
http://
Executed test runs:
SUCCESS: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:182
http://
Executed test runs:
SUCCESS: http://
Click here to trigger a rebuild:
http://
- 183. By Robert Bruce Park
-
Regex is overkill.
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:183
http://
Executed test runs:
SUCCESS: http://
Click here to trigger a rebuild:
http://
- 184. By Robert Bruce Park
-
Rebase on trunk.
- 185. By Robert Bruce Park
-
Flush revision queue every 5 minutes.
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:184
http://
Executed test runs:
SUCCESS: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:185
http://
Executed test runs:
SUCCESS: http://
Click here to trigger a rebuild:
http://
Preview Diff
1 | === added file 'data/model-schema.csv' |
2 | --- data/model-schema.csv 1970-01-01 00:00:00 +0000 |
3 | +++ data/model-schema.csv 2013-04-05 18:49:21 +0000 |
4 | @@ -0,0 +1,23 @@ |
5 | +protocol,s |
6 | +account_id,t |
7 | +message_id,s |
8 | +stream,s |
9 | +sender,s |
10 | +sender_id,s |
11 | +sender_nick,s |
12 | +from_me,b |
13 | +timestamp,s |
14 | +message,s |
15 | +icon_uri,s |
16 | +url,s |
17 | +likes,t |
18 | +liked,b |
19 | +link_picture,s |
20 | +link_name,s |
21 | +link_url,s |
22 | +link_desc,s |
23 | +link_caption,s |
24 | +link_icon,s |
25 | +location,s |
26 | +latitude,d |
27 | +longitude,d |
28 | |
29 | === modified file 'debian/friends.install' |
30 | --- debian/friends.install 2013-02-07 22:05:09 +0000 |
31 | +++ debian/friends.install 2013-04-05 18:49:21 +0000 |
32 | @@ -1,3 +1,4 @@ |
33 | usr/bin/friends-service |
34 | usr/share/dbus-1/services/com.canonical.Friends.Service.service |
35 | +usr/share/friends/model-schema.csv |
36 | usr/share/glib-2.0/schemas/com.canonical.friends.gschema.xml |
37 | |
38 | === modified file 'friends/tests/mocks.py' |
39 | --- friends/tests/mocks.py 2013-03-14 19:44:13 +0000 |
40 | +++ friends/tests/mocks.py 2013-04-05 18:49:21 +0000 |
41 | @@ -40,9 +40,16 @@ |
42 | from urllib.parse import urlsplit |
43 | from gi.repository import Dee |
44 | |
45 | +# By default, Schema.FILES will look for the system-installed schema |
46 | +# file first, and then failing that will look for the one in the |
47 | +# source tree, for performance reasons. During testing though, we want |
48 | +# to look at the source tree first, so we reverse the list here. |
49 | +from friends.utils.model import Schema |
50 | +Schema.FILES = list(reversed(Schema.FILES)) |
51 | +SCHEMA = Schema() |
52 | + |
53 | from friends.utils.base import Base |
54 | from friends.utils.logging import LOG_FORMAT |
55 | -from friends.utils.model import COLUMN_TYPES |
56 | |
57 | |
58 | try: |
59 | @@ -58,7 +65,7 @@ |
60 | # Create a test model that will not interfere with the user's environment. |
61 | # We'll use this object as a mock of the real model. |
62 | TestModel = Dee.SharedModel.new('com.canonical.Friends.TestSharedModel') |
63 | -TestModel.set_schema_full(COLUMN_TYPES) |
64 | +TestModel.set_schema_full(SCHEMA.TYPES) |
65 | |
66 | |
67 | @mock.patch('friends.utils.http._soup', mock.Mock()) |
68 | |
69 | === modified file 'friends/tests/test_protocols.py' |
70 | --- friends/tests/test_protocols.py 2013-04-04 19:44:01 +0000 |
71 | +++ friends/tests/test_protocols.py 2013-04-05 18:49:21 +0000 |
72 | @@ -26,10 +26,10 @@ |
73 | |
74 | from friends.protocols.flickr import Flickr |
75 | from friends.protocols.twitter import Twitter |
76 | -from friends.tests.mocks import FakeAccount, LogMock, TestModel, mock |
77 | +from friends.tests.mocks import SCHEMA, FakeAccount, LogMock, TestModel, mock |
78 | from friends.utils.base import Base, feature, linkify_string |
79 | from friends.utils.manager import ProtocolManager |
80 | -from friends.utils.model import COLUMN_INDICES, Model |
81 | +from friends.utils.model import Model |
82 | |
83 | |
84 | class TestProtocolManager(unittest.TestCase): |
85 | @@ -282,7 +282,7 @@ |
86 | self.assertEqual(1, TestModel.get_n_rows()) |
87 | # The first published message wins. |
88 | row = TestModel.get_row(0) |
89 | - self.assertEqual(row[COLUMN_INDICES['message']], 'hello, @jimmy') |
90 | + self.assertEqual(row[SCHEMA.INDICES['message']], 'hello, @jimmy') |
91 | |
92 | @mock.patch('friends.utils.base.Model', TestModel) |
93 | @mock.patch('friends.utils.base._seen_ids', {}) |
94 | @@ -364,10 +364,10 @@ |
95 | # See? Two rows in the table. |
96 | self.assertEqual(2, TestModel.get_n_rows()) |
97 | # The first row is the message from fred. |
98 | - self.assertEqual(TestModel.get_row(0)[COLUMN_INDICES['sender']], |
99 | + self.assertEqual(TestModel.get_row(0)[SCHEMA.INDICES['sender']], |
100 | 'fred') |
101 | # The second row is the message from tedtholomew. |
102 | - self.assertEqual(TestModel.get_row(1)[COLUMN_INDICES['sender']], |
103 | + self.assertEqual(TestModel.get_row(1)[SCHEMA.INDICES['sender']], |
104 | 'tedtholomew') |
105 | |
106 | @mock.patch('friends.utils.base.Model', TestModel) |
107 | |
108 | === modified file 'friends/utils/base.py' |
109 | --- friends/utils/base.py 2013-04-04 22:33:23 +0000 |
110 | +++ friends/utils/base.py 2013-04-05 18:49:21 +0000 |
111 | @@ -35,22 +35,22 @@ |
112 | |
113 | from friends.errors import FriendsError, ContactsError |
114 | from friends.utils.authentication import Authentication |
115 | -from friends.utils.model import COLUMN_INDICES, SCHEMA, DEFAULTS |
116 | -from friends.utils.model import Model, persist_model |
117 | +from friends.utils.model import Schema, Model, persist_model |
118 | from friends.utils.notify import notify |
119 | from friends.utils.time import ISO8601_FORMAT |
120 | |
121 | |
122 | STUB = lambda *ignore, **kwignore: None |
123 | COMMA_SPACE = ', ' |
124 | -AVATAR_IDX = COLUMN_INDICES['icon_uri'] |
125 | -FROM_ME_IDX = COLUMN_INDICES['from_me'] |
126 | -STREAM_IDX = COLUMN_INDICES['stream'] |
127 | -SENDER_IDX = COLUMN_INDICES['sender'] |
128 | -MESSAGE_IDX = COLUMN_INDICES['message'] |
129 | -ID_IDX = COLUMN_INDICES['message_id'] |
130 | -ACCT_IDX = COLUMN_INDICES['account_id'] |
131 | -TIME_IDX = COLUMN_INDICES['timestamp'] |
132 | +SCHEMA = Schema() |
133 | +AVATAR_IDX = SCHEMA.INDICES['icon_uri'] |
134 | +FROM_ME_IDX = SCHEMA.INDICES['from_me'] |
135 | +STREAM_IDX = SCHEMA.INDICES['stream'] |
136 | +SENDER_IDX = SCHEMA.INDICES['sender'] |
137 | +MESSAGE_IDX = SCHEMA.INDICES['message'] |
138 | +ID_IDX = SCHEMA.INDICES['message_id'] |
139 | +ACCT_IDX = SCHEMA.INDICES['account_id'] |
140 | +TIME_IDX = SCHEMA.INDICES['timestamp'] |
141 | |
142 | # See friends/tests/test_protocols.py for further documentation |
143 | LINKIFY_REGEX = re.compile( |
144 | @@ -355,8 +355,8 @@ |
145 | # the order which they appear in the SCHEMA. If any are left |
146 | # over at the end of this, raise a TypeError indicating the |
147 | # unexpected column names. |
148 | - for column_name, column_type in SCHEMA: |
149 | - args.append(kwargs.pop(column_name, DEFAULTS[column_type])) |
150 | + for column_name, column_type in SCHEMA.COLUMNS: |
151 | + args.append(kwargs.pop(column_name, SCHEMA.DEFAULTS[column_type])) |
152 | if len(kwargs) > 0: |
153 | raise TypeError('Unexpected keyword arguments: {}'.format( |
154 | COMMA_SPACE.join(sorted(kwargs)))) |
155 | @@ -506,7 +506,7 @@ |
156 | def _calculate_row_cell(self, message_id, column_name): |
157 | """Find x,y coords in the model based on message_id and column_name.""" |
158 | row_id = _seen_ids.get(message_id) |
159 | - col_idx = COLUMN_INDICES.get(column_name) |
160 | + col_idx = SCHEMA.INDICES.get(column_name) |
161 | if None in (row_id, col_idx): |
162 | raise FriendsError('Cell could not be found.') |
163 | return row_id, col_idx |
164 | |
165 | === modified file 'friends/utils/model.py' |
166 | --- friends/utils/model.py 2013-03-13 04:36:33 +0000 |
167 | +++ friends/utils/model.py 2013-04-05 18:49:21 +0000 |
168 | @@ -24,11 +24,8 @@ |
169 | |
170 | |
171 | __all__ = [ |
172 | + 'Schema', |
173 | 'Model', |
174 | - 'COLUMN_NAMES', |
175 | - 'COLUMN_TYPES', |
176 | - 'COLUMN_INDICES', |
177 | - 'DEFAULTS', |
178 | 'MODEL_DBUS_NAME', |
179 | 'persist_model', |
180 | 'prune_model', |
181 | @@ -41,46 +38,44 @@ |
182 | log = logging.getLogger(__name__) |
183 | |
184 | |
185 | -# DO NOT EDIT THIS WITHOUT ADJUSTING service.vala IN LOCKSTEP |
186 | -SCHEMA = ( |
187 | - ('protocol', 's'), # Same as UOA 'provider_name' |
188 | - ('account_id', 't'), # Same as UOA account id |
189 | - ('message_id', 's'), |
190 | - ('stream', 's'), |
191 | - ('sender', 's'), |
192 | - ('sender_id', 's'), |
193 | - ('sender_nick', 's'), |
194 | - ('from_me', 'b'), |
195 | - ('timestamp', 's'), |
196 | - ('message', 's'), |
197 | - ('icon_uri', 's'), |
198 | - ('url', 's'), |
199 | - ('likes', 't'), |
200 | - ('liked', 'b'), |
201 | - ('link_picture', 's'), |
202 | - ('link_name', 's'), |
203 | - ('link_url', 's'), |
204 | - ('link_desc', 's'), |
205 | - ('link_caption', 's'), |
206 | - ('link_icon', 's'), |
207 | - ('location', 's'), |
208 | - ('latitude', 'd'), |
209 | - ('longitude', 'd'), |
210 | - ) |
211 | - |
212 | - |
213 | -# It's useful to have separate lists of the column names and types. |
214 | -COLUMN_NAMES, COLUMN_TYPES = zip(*SCHEMA) |
215 | -# A reverse mapping from column name to the column index. This is useful for |
216 | -# pulling column values out of a row of data. |
217 | -COLUMN_INDICES = {name: i for i, name in enumerate(COLUMN_NAMES)} |
218 | -# This defines default values for the different data types |
219 | -DEFAULTS = { |
220 | - 'b': False, |
221 | - 's': '', |
222 | - 'd': 0, |
223 | - 't': 0, |
224 | - } |
225 | +class Schema: |
226 | + """Represents the DeeModel schema data that we defined in CSV.""" |
227 | + DEFAULTS = { |
228 | + 'b': False, |
229 | + 's': '', |
230 | + 'd': 0, |
231 | + 't': 0, |
232 | + } |
233 | + |
234 | + FILES = [ |
235 | + 'data/model-schema.csv', |
236 | + '/usr/share/friends/model-schema.csv', |
237 | + ] |
238 | + |
239 | + def __init__(self): |
240 | + """Parse CSV from disk.""" |
241 | + self.COLUMNS = [] |
242 | + self.NAMES = [] |
243 | + self.TYPES = [] |
244 | + self.INDICES = {} |
245 | + |
246 | + files = self.FILES.copy() |
247 | + while files: |
248 | + filename = files.pop() |
249 | + log.debug('Looking for SCHEMA in {}'.format(filename)) |
250 | + try: |
251 | + with open(filename) as schema: |
252 | + for col in schema: |
253 | + name, variant = col.rstrip().split(',') |
254 | + self.COLUMNS.append((name, variant)) |
255 | + self.NAMES.append(name) |
256 | + self.TYPES.append(variant) |
257 | + log.debug( |
258 | + 'Found {} columns for SCHEMA'.format(len(self.COLUMNS))) |
259 | + break |
260 | + except IOError: |
261 | + pass |
262 | + self.INDICES = {name: i for i, name in enumerate(self.NAMES)} |
263 | |
264 | |
265 | MODEL_DBUS_NAME = 'com.canonical.Friends.Streams' |
266 | |
267 | === modified file 'service/src/service.vala' |
268 | --- service/src/service.vala 2013-03-13 04:36:33 +0000 |
269 | +++ service/src/service.vala 2013-04-05 18:49:21 +0000 |
270 | @@ -74,29 +74,17 @@ |
271 | debug ("Failed to load model from resource manager: %s", e.message); |
272 | } |
273 | |
274 | - string[] SCHEMA = {"s", |
275 | - "t", |
276 | - "s", |
277 | - "s", |
278 | - "s", |
279 | - "s", |
280 | - "s", |
281 | - "b", |
282 | - "s", |
283 | - "s", |
284 | - "s", |
285 | - "s", |
286 | - "t", |
287 | - "b", |
288 | - "s", |
289 | - "s", |
290 | - "s", |
291 | - "s", |
292 | - "s", |
293 | - "s", |
294 | - "s", |
295 | - "d", |
296 | - "d"}; |
297 | + string[] SCHEMA = {}; |
298 | + |
299 | + var file = FileStream.open("/usr/share/friends/model-schema.csv", "r"); |
300 | + string line = null; |
301 | + while (true) |
302 | + { |
303 | + line = file.read_line(); |
304 | + if (line == null) break; |
305 | + SCHEMA += line.split(",")[1]; |
306 | + } |
307 | + debug ("Found %u schema columns.", SCHEMA.length); |
308 | |
309 | bool schemaReset = false; |
310 | |
311 | @@ -109,7 +97,7 @@ |
312 | schemaReset = true; |
313 | else |
314 | { |
315 | - for (int i=0; i < _SCHEMA.length;i++ ) |
316 | + for (int i=0; i < _SCHEMA.length; i++) |
317 | { |
318 | if (_SCHEMA[i] != SCHEMA[i]) |
319 | { |
320 | @@ -165,11 +153,11 @@ |
321 | debug ("NOT LEADER"); |
322 | }); |
323 | |
324 | - Timeout.add_seconds (30, () => { |
325 | + Timeout.add_seconds (300, () => { |
326 | shared_model.flush_revision_queue(); |
327 | debug ("Storing model with %u rows", model.get_n_rows()); |
328 | resources.store ((Dee.SequenceModel)model, "com.canonical.Friends.Streams"); |
329 | - return false; |
330 | + return true; |
331 | }); |
332 | } |
333 | |
334 | |
335 | === modified file 'setup.py' |
336 | --- setup.py 2013-03-14 20:25:42 +0000 |
337 | +++ setup.py 2013-04-05 18:49:21 +0000 |
338 | @@ -33,7 +33,9 @@ |
339 | }, |
340 | data_files = [ |
341 | ('/usr/share/glib-2.0/schemas', |
342 | - ['data/com.canonical.friends.gschema.xml']) |
343 | + ['data/com.canonical.friends.gschema.xml']), |
344 | + ('/usr/share/friends', |
345 | + ['data/model-schema.csv']), |
346 | ], |
347 | entry_points = { |
348 | 'console_scripts': ['friends-dispatcher = friends.main:main'], |
349 | |
350 | === modified file 'tools/debug_live.py' |
351 | --- tools/debug_live.py 2013-03-13 18:05:13 +0000 |
352 | +++ tools/debug_live.py 2013-04-05 18:49:21 +0000 |
353 | @@ -24,6 +24,9 @@ |
354 | |
355 | sys.path.insert(0, '.') |
356 | |
357 | +# Ignore system-installed schema. |
358 | +from friends.tests.mocks import SCHEMA |
359 | + |
360 | from gi.repository import GLib |
361 | from friends.utils.logging import initialize |
362 | |
363 | |
364 | === modified file 'tools/debug_slave.py' |
365 | --- tools/debug_slave.py 2013-04-04 22:33:06 +0000 |
366 | +++ tools/debug_slave.py 2013-04-05 18:49:21 +0000 |
367 | @@ -11,7 +11,8 @@ |
368 | from gi.repository import Dee |
369 | from gi.repository import GLib |
370 | |
371 | -from friends.utils.model import COLUMN_NAMES |
372 | +from friends.tests.mocks import SCHEMA |
373 | + |
374 | |
375 | class Slave: |
376 | def __init__(self): |
377 | @@ -25,7 +26,7 @@ |
378 | row = self.model.get_row(itr) |
379 | print('\n' * 5) |
380 | for i, col in enumerate(row): |
381 | - print('{:12}: {}'.format(COLUMN_NAMES[i], col)) |
382 | + print('{:12}: {}'.format(SCHEMA.NAMES[i], col)) |
383 | print('ROWS: ', len(self.model)) |
384 | |
385 |
PASSED: Continuous integration, rev:179 jenkins. qa.ubuntu. com/job/ friends- ci/19/ jenkins. qa.ubuntu. com/job/ friends- raring- amd64-ci/ 19
http://
Executed test runs:
SUCCESS: http://
Click here to trigger a rebuild: s-jenkins: 8080/job/ friends- ci/19/rebuild
http://