Launchpad itself

Merge lp:~cr3/launchpad/hwsubmissionset_search into lp:launchpad/db-devel

  1. hwsubmissionset_search
  2. Merge into db-devel
Proposed by Marc Tardif
Status: Merged
Approved by: Robert Collins
Approved revision: no longer in the source branch.
Merged at revision: 10711
Proposed branch: lp:~cr3/launchpad/hwsubmissionset_search
Merge into: lp:launchpad/db-devel
Diff against target: 324 lines (+241/-3)
6 files modified
database/schema/patch-2208-73-2.sql (+12/-0)
lib/canonical/launchpad/interfaces/_schema_circular_imports.py (+1/-1)
lib/canonical/launchpad/systemhomes.py (+13/-0)
lib/lp/hardwaredb/interfaces/hwdb.py (+110/-1)
lib/lp/hardwaredb/model/hwdb.py (+11/-1)
lib/lp/hardwaredb/stories/webservice/xx-hwdb.txt (+94/-0)
To merge this branch: bzr merge lp:~cr3/launchpad/hwsubmissionset_search
Reviewer Review Type Date Requested Status
Robert Collins (community) Approve
Stuart Bishop (community) db Approve
Canonical Launchpad Engineering Pending
Review via email: mp+63768@code.launchpad.net

Commit message

[r=lifeless,stub][bug=801334] Exposing IHWSubmissionSet.search in the API and extending it to search by date for integration with the Results Tracker LEP. (Landed by bac on behalf of cr3)

Description of the change

Added two indexes to the hwsubmission table to account for searching hwsubmissions by date_created and date_submitted respectively. No sampledata was necessary.

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

Minor typo - the second index has '_idx' in its name instead of '__idx' with two underscores.

Otherwise fine. patch-2208-74-0.sql

Rename the db patch to that name, change the LaunchpadDatabaseRevision line to match and you are good to land from a db pov.

review: Approve (db)
Revision history for this message
Stuart Bishop (stub) wrote :

The indexes can also be created live if we need this active sooner than next month's rollout. There is more branch juggling to do though, as your code needs to land on launchpad/devel and the db patch (with a different number) landed on launchpad/db-devel.

Revision history for this message
Stuart Bishop (stub) wrote :

I've just added a review request for the code to be reviewed.

Revision history for this message
Marc Tardif (cr3) wrote :

Applied all the changes you requested and pushed to the same branch. As for creating the indexes live, your offer is much appreciated but I'll first evaluate the performance on the production database once the code is deployed. If I get timeouts, then I'll consider your offer. Thanks!

Revision history for this message
Robert Collins (lifeless) wrote :

Code wise - a few thoughts.

The function signature you've added confuses me.
What does this mean:
+ :param submitted_before: Limit results to submissions submitted
78 + before this date inclusively.
79 + :param submitted_after: Limit results to submissions submitted
80 + after this date exclusively.

The inclusive and exclusive bits could refer to being a half-open or closed range endpoint, or to the way that the constraint is connected to the rest of the search.

Perhaps a better thing to say is 'Exclude results submitted after this date'; 'Exclude results submitted before or on this date'

There is one little nit on style, which you can ignore if you like (though the next person through will likely tidy it up: vertical whitespace in a function is frowned upon in PEP8: '
    Use blank lines in functions, sparingly, to indicate logical sections.
'

The fact you're extending a doctest is killing baby kittens in alaska. Did you consider unit testing this?

Stub, we could make this a -1 patch can't we? That way it can all land on devel and we can do the indices live? hwsubmissions are our current #1 timeout...

review: Approve
Revision history for this message
Stuart Bishop (stub) wrote :

> Otherwise fine. patch-2208-74-0.sql
>
> Rename the db patch to that name, change the LaunchpadDatabaseRevision line to
> match and you are good to land from a db pov.

Please use the patch number 2208-73-1.sql instead. This will fit in better with our deployment streamlining work.

Revision history for this message
Robert Collins (lifeless) wrote :

@Marc once you do stuarts last change and address the clarity stuff I mentioned we can land this; I've marked it approved - their is no need to go through another review round-trip (but if you can't land it yourself, please do ping an lp dev once you've made the changes to do the landing for you).

Revision history for this message
Marc Tardif (cr3) wrote :

> Perhaps a better thing to say is 'Exclude results submitted after this
> date'; 'Exclude results submitted before or on this date'

Done.

> Use blank lines in functions, sparingly, to indicate logical sections.

I typically consider each conditional statement as a logical section but
I can appreciate how this can result in lots of vertical noise, so I've
increased the signal.

> The fact you're extending a doctest is killing baby kittens in
> alaska. Did you consider unit testing this?

I was torn between writing a new set of unit tests or extending the
existing code base which was relying on doctests. I agree the former
would've been preferable but would've been a much more significant
undertaking than the patch itself. So, if the hwdb is likely to get
more attention, then I think it'll be worthwhile to move the existing
doctests to unit tests and save a few kittens in the process.

> Stub, we could make this a -1 patch can't we? That way it can all land
> on devel and we can do the indices live? hwsubmissions are our current
> #1 timeout...

I renamed the patch following stub's comment and made sure to update
LaunchpadDatabaseRevision accordingly. Finally, I just pinged someone
in #launchpad-dev to help with landing the patch.

Thanks for all the advice!

Revision history for this message
Stuart Bishop (stub) wrote :

Bah. I need to improve my nodes.

patch-2208-73-2.sql

Sorry about that. Working out bugs in the new system :)

Revision history for this message
Marc Tardif (cr3) wrote :

Done, renamed the patch again and made sure to update LaunchpadDatabaseRevision accordingly.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added file 'database/schema/patch-2208-73-2.sql'
2--- database/schema/patch-2208-73-2.sql 1970-01-01 00:00:00 +0000
3+++ database/schema/patch-2208-73-2.sql 2011-06-16 13:11:48 +0000
4@@ -0,0 +1,12 @@
5+-- Copyright 2011 Canonical Ltd. This software is licensed under the
6+-- GNU Affero General Public License version 3 (see the file LICENSE).
7+
8+SET client_min_messages=ERROR;
9+
10+-- For IHWSubmissionSet, which can now search by date_created.
11+CREATE INDEX hwsubmission__date_created__idx ON hwsubmission USING btree (date_created);
12+
13+-- For IHWSubmissionSet, which can now search by date_submitted.
14+CREATE INDEX hwsubmission__date_submitted__idx ON hwsubmission USING btree (date_submitted);
15+
16+INSERT INTO LaunchpadDatabaseRevision VALUES (2208, 73, 2);
17
18=== modified file 'lib/canonical/launchpad/interfaces/_schema_circular_imports.py'
19--- lib/canonical/launchpad/interfaces/_schema_circular_imports.py 2011-06-11 00:49:33 +0000
20+++ lib/canonical/launchpad/interfaces/_schema_circular_imports.py 2011-06-16 13:11:48 +0000
21@@ -894,7 +894,7 @@
22 patch_operations_explicit_version(
23 IHWDBApplication, 'beta', "deviceDriverOwnersAffectedByBugs", "devices",
24 "drivers", "hwInfoByBugRelatedUsers", "numDevicesInSubmissions",
25- "numOwnersOfDevice", "numSubmissionsWithDevice", "vendorIDs")
26+ "numOwnersOfDevice", "numSubmissionsWithDevice", "search", "vendorIDs")
27
28 # IHWDevice
29 patch_entry_explicit_version(IHWDevice, 'beta')
30
31=== modified file 'lib/canonical/launchpad/systemhomes.py'
32--- lib/canonical/launchpad/systemhomes.py 2011-01-21 21:42:40 +0000
33+++ lib/canonical/launchpad/systemhomes.py 2011-06-16 13:11:48 +0000
34@@ -296,6 +296,19 @@
35 """See `IHWDBApplication`."""
36 return getUtility(IHWDriverSet).all_package_names()
37
38+ def search(self, user=None, device=None, driver=None, distribution=None,
39+ distroseries=None, architecture=None, owner=None,
40+ created_before=None, created_after=None,
41+ submitted_before=None, submitted_after=None):
42+ """See `IHWDBApplication`."""
43+ return getUtility(IHWSubmissionSet).search(
44+ user=user, device=device, driver=driver,
45+ distribution=distribution, distroseries=distroseries,
46+ architecture=architecture, owner=owner,
47+ created_before=created_before, created_after=created_after,
48+ submitted_before=submitted_before,
49+ submitted_after=submitted_after)
50+
51 def getDistroTarget(self, distribution, distroseries, distroarchseries):
52 distro_targets = [
53 target for target in (
54
55=== modified file 'lib/lp/hardwaredb/interfaces/hwdb.py'
56--- lib/lp/hardwaredb/interfaces/hwdb.py 2011-02-24 15:30:54 +0000
57+++ lib/lp/hardwaredb/interfaces/hwdb.py 2011-06-16 13:11:48 +0000
58@@ -283,7 +283,9 @@
59 """
60
61 def search(user=None, device=None, driver=None, distribution=None,
62- distroseries=None, architecture=None, owner=None):
63+ distroseries=None, architecture=None, owner=None,
64+ created_before=None, created_after=None,
65+ submitted_before=None, submitted_after=None):
66 """Return the submissions matiching the given parmeters.
67
68 :param user: The `IPerson` running the query. Private submissions
69@@ -300,6 +302,14 @@
70 :param architecture: Limit results to submissions made for
71 a specific architecture.
72 :param owner: Limit results to submissions from this person.
73+ :param created_before: Exclude results created after this
74+ date.
75+ :param created_after: Exclude results created before or on
76+ this date.
77+ :param submitted_before: Exclude results submitted after this
78+ date.
79+ :param submitted_after: Exclude results submitted before or on
80+ this date.
81
82 Only one of :distribution: or :distroseries: may be supplied.
83 """
84@@ -1235,6 +1245,105 @@
85 readonly=True))
86
87 @operation_parameters(
88+ device=Reference(
89+ IHWDevice,
90+ title=u'A Device',
91+ description=(
92+ u'If specified, the result set is limited to submissions '
93+ u'containing this device.'),
94+ required=False),
95+ driver=Reference(
96+ IHWDriver,
97+ title=u'A Driver',
98+ description=(
99+ u'If specified, the result set is limited to submissions '
100+ u'containing devices that use this driver.'),
101+ required=False),
102+ distribution=Reference(
103+ IDistribution,
104+ title=u'A Distribution',
105+ description=(
106+ u'If specified, the result set is limited to submissions '
107+ u'made for this distribution.'),
108+ required=False),
109+ distroseries=Reference(
110+ IDistroSeries,
111+ title=u'A Distribution Series',
112+ description=(
113+ u'If specified, the result set is limited to submissions '
114+ u'made for the given distribution series.'),
115+ required=False),
116+ architecture=TextLine(
117+ title=u'A processor architecture',
118+ description=
119+ u'If specified, the result set is limited to sumbissions '
120+ 'made for a specific architecture.',
121+ required=False),
122+ owner=Reference(
123+ IPerson,
124+ title=u'Person',
125+ description=
126+ u'If specified, the result set is limited to sumbissions '
127+ 'from this person.',
128+ required=False),
129+ created_before=Datetime(
130+ title=u'Created Before',
131+ description=
132+ u'If specified, exclude results created after this date.',
133+ required=False),
134+ created_after=Datetime(
135+ title=u'Created After',
136+ description=
137+ u'If specified, exclude results created before or on '
138+ 'this date.',
139+ required=False),
140+ submitted_before=Datetime(
141+ title=u'Created Before',
142+ description=
143+ u'If specified, exclude results submitted after this date.',
144+ required=False),
145+ submitted_after=Datetime(
146+ title=u'Created After',
147+ description=
148+ u'If specified, Exclude results submitted before or on '
149+ 'this date.',
150+ required=False))
151+ @call_with(user=REQUEST_USER)
152+ @operation_returns_collection_of(IHWSubmission)
153+ @export_read_operation()
154+ def search(user=None, device=None, driver=None, distribution=None,
155+ distroseries=None, architecture=None, owner=None,
156+ created_before=None, created_after=None,
157+ submitted_before=None, submitted_after=None):
158+ """Return the submissions matiching the given parmeters.
159+
160+ :param user: The `IPerson` running the query. Private submissions
161+ are returned only if the person running the query is the
162+ owner or an admin.
163+ :param device: Limit results to submissions containing this
164+ `IHWDevice`.
165+ :param driver: Limit results to submissions containing devices
166+ that use this `IHWDriver`.
167+ :param distribution: Limit results to submissions made for
168+ this `IDistribution`.
169+ :param distroseries: Limit results to submissions made for
170+ this `IDistroSeries`.
171+ :param architecture: Limit results to submissions made for
172+ a specific architecture.
173+ :param owner: Limit results to submissions from this person.
174+ :param created_before: Exclude results created after this
175+ date.
176+ :param created_after: Exclude results created before or on
177+ this date.
178+ :param submitted_before: Exclude results submitted after this
179+ date.
180+ :param submitted_after: Exclude results submitted before or on
181+ this date.
182+
183+ Only one of :distribution: or :distroseries: may be supplied.
184+ """
185+
186+ @operation_parameters(
187 bus=Choice(
188 title=u'The device bus', vocabulary=HWBus, required=False),
189 vendor_id=TextLine(
190
191=== modified file 'lib/lp/hardwaredb/model/hwdb.py'
192--- lib/lp/hardwaredb/model/hwdb.py 2011-05-27 21:12:25 +0000
193+++ lib/lp/hardwaredb/model/hwdb.py 2011-06-16 13:11:48 +0000
194@@ -294,7 +294,9 @@
195 return result_set
196
197 def search(self, user=None, device=None, driver=None, distribution=None,
198- distroseries=None, architecture=None, owner=None):
199+ distroseries=None, architecture=None, owner=None,
200+ created_before=None, created_after=None,
201+ submitted_before=None, submitted_after=None):
202 """See `IHWSubmissionSet`."""
203 store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
204 args = []
205@@ -330,6 +332,14 @@
206 args.append(DistroArchSeries.distroseries == distroseries.id)
207 if owner is not None:
208 args.append(HWSubmission.owner == owner.id)
209+ if created_before is not None:
210+ args.append(HWSubmission.date_created <= created_before)
211+ if created_after is not None:
212+ args.append(HWSubmission.date_created > created_after)
213+ if submitted_before is not None:
214+ args.append(HWSubmission.date_submitted <= submitted_before)
215+ if submitted_after is not None:
216+ args.append(HWSubmission.date_submitted > submitted_after)
217
218 result_set = store.find(
219 HWSubmission,
220
221=== modified file 'lib/lp/hardwaredb/stories/webservice/xx-hwdb.txt'
222--- lib/lp/hardwaredb/stories/webservice/xx-hwdb.txt 2011-02-13 22:54:48 +0000
223+++ lib/lp/hardwaredb/stories/webservice/xx-hwdb.txt 2011-06-16 13:11:48 +0000
224@@ -711,6 +711,100 @@
225 ...
226
227
228+=== Searching for submissions ===
229+
230+Alternatively, we can also search for hardware submissions by user:
231+
232+ >>> owner = webservice.getAbsoluteUrl('/~name12')
233+ >>> submissions = webservice.get(
234+ ... '/+hwdb?ws.op=search&owner=%s' % owner).jsonBody()
235+ >>> print submissions['total_size']
236+ 2
237+
238+ >>> owner = webservice.getAbsoluteUrl('/~name20')
239+ >>> submissions = webservice.get(
240+ ... '/+hwdb?ws.op=search&owner=%s' % owner).jsonBody()
241+ >>> print submissions['total_size']
242+ 0
243+
244+...and by device:
245+
246+ >>> device = webservice.getAbsoluteUrl('/+hwdb/+device/1')
247+ >>> submissions = webservice.named_get(
248+ ... '/+hwdb', 'search', device=device).jsonBody()
249+ >>> print submissions['total_size']
250+ 1
251+
252+...and by driver:
253+
254+ >>> driver = webservice.getAbsoluteUrl('/+hwdb/+driver/1')
255+ >>> submissions = webservice.named_get(
256+ ... '/+hwdb', 'search', driver=driver).jsonBody()
257+ >>> print submissions['total_size']
258+ 1
259+
260+...and by distribution:
261+
262+ >>> ubuntu_url = webservice.getAbsoluteUrl('/ubuntu')
263+ >>> submissions = webservice.named_get(
264+ ... '/+hwdb', 'search', distribution=ubuntu_url).jsonBody()
265+ >>> print submissions['total_size']
266+ 1
267+ >>> debian_url = webservice.getAbsoluteUrl('/debian')
268+ >>> submissions = webservice.named_get(
269+ ... '/+hwdb', 'search', distribution=debian_url).jsonBody()
270+ >>> print submissions['total_size']
271+ 0
272+
273+...and by distroseries:
274+
275+ >>> hoary_url = webservice.getAbsoluteUrl('/ubuntu/hoary')
276+ >>> submissions = webservice.named_get(
277+ ... '/+hwdb', 'search', distroseries=hoary_url).jsonBody()
278+ >>> print submissions['total_size']
279+ 1
280+ >>> warty_url = webservice.getAbsoluteUrl('/ubuntu/warty')
281+ >>> submissions = webservice.named_get(
282+ ... '/+hwdb', 'search', distroseries=warty_url).jsonBody()
283+ >>> print submissions['total_size']
284+ 0
285+
286+...and by architecture:
287+
288+ >>> submissions = webservice.named_get(
289+ ... '/+hwdb', 'search', architecture='i386').jsonBody()
290+ >>> print submissions['total_size']
291+ 1
292+ >>> submissions = webservice.named_get(
293+ ... '/+hwdb', 'search', architecture='powerpc').jsonBody()
294+ >>> print submissions['total_size']
295+ 0
296+
297+...and by date created:
298+
299+ >>> date_created = u'2007-09-11T00:00:00+00:00'
300+ >>> submissions = webservice.named_get(
301+ ... '/+hwdb', 'search', created_before=date_created).jsonBody()
302+ >>> print submissions['total_size']
303+ 1
304+ >>> submissions = webservice.named_get(
305+ ... '/+hwdb', 'search', created_after=date_created).jsonBody()
306+ >>> print submissions['total_size']
307+ 1
308+
309+...and by date submitted:
310+
311+ >>> date_submitted = u'2007-09-11T15:23:45.653316+00:00'
312+ >>> submissions = webservice.named_get(
313+ ... '/+hwdb', 'search', submitted_before=date_submitted).jsonBody()
314+ >>> print submissions['total_size']
315+ 1
316+ >>> submissions = webservice.named_get(
317+ ... '/+hwdb', 'search', submitted_after=date_submitted).jsonBody()
318+ >>> print submissions['total_size']
319+ 1
320+
321+
322 === Submission Devices ===
323
324 The table HWSubmissionDevice associates devices with submissions.

Subscribers

People subscribed via source and target branches

to status/vote changes: