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
=== added file 'database/schema/patch-2208-73-2.sql'
--- database/schema/patch-2208-73-2.sql 1970-01-01 00:00:00 +0000
+++ database/schema/patch-2208-73-2.sql 2011-06-16 13:11:48 +0000
@@ -0,0 +1,12 @@
1-- Copyright 2011 Canonical Ltd. This software is licensed under the
2-- GNU Affero General Public License version 3 (see the file LICENSE).
3
4SET client_min_messages=ERROR;
5
6-- For IHWSubmissionSet, which can now search by date_created.
7CREATE INDEX hwsubmission__date_created__idx ON hwsubmission USING btree (date_created);
8
9-- For IHWSubmissionSet, which can now search by date_submitted.
10CREATE INDEX hwsubmission__date_submitted__idx ON hwsubmission USING btree (date_submitted);
11
12INSERT INTO LaunchpadDatabaseRevision VALUES (2208, 73, 2);
013
=== modified file 'lib/canonical/launchpad/interfaces/_schema_circular_imports.py'
--- lib/canonical/launchpad/interfaces/_schema_circular_imports.py 2011-06-11 00:49:33 +0000
+++ lib/canonical/launchpad/interfaces/_schema_circular_imports.py 2011-06-16 13:11:48 +0000
@@ -894,7 +894,7 @@
894patch_operations_explicit_version(894patch_operations_explicit_version(
895 IHWDBApplication, 'beta', "deviceDriverOwnersAffectedByBugs", "devices",895 IHWDBApplication, 'beta', "deviceDriverOwnersAffectedByBugs", "devices",
896 "drivers", "hwInfoByBugRelatedUsers", "numDevicesInSubmissions",896 "drivers", "hwInfoByBugRelatedUsers", "numDevicesInSubmissions",
897 "numOwnersOfDevice", "numSubmissionsWithDevice", "vendorIDs")897 "numOwnersOfDevice", "numSubmissionsWithDevice", "search", "vendorIDs")
898898
899# IHWDevice899# IHWDevice
900patch_entry_explicit_version(IHWDevice, 'beta')900patch_entry_explicit_version(IHWDevice, 'beta')
901901
=== modified file 'lib/canonical/launchpad/systemhomes.py'
--- lib/canonical/launchpad/systemhomes.py 2011-01-21 21:42:40 +0000
+++ lib/canonical/launchpad/systemhomes.py 2011-06-16 13:11:48 +0000
@@ -296,6 +296,19 @@
296 """See `IHWDBApplication`."""296 """See `IHWDBApplication`."""
297 return getUtility(IHWDriverSet).all_package_names()297 return getUtility(IHWDriverSet).all_package_names()
298298
299 def search(self, user=None, device=None, driver=None, distribution=None,
300 distroseries=None, architecture=None, owner=None,
301 created_before=None, created_after=None,
302 submitted_before=None, submitted_after=None):
303 """See `IHWDBApplication`."""
304 return getUtility(IHWSubmissionSet).search(
305 user=user, device=device, driver=driver,
306 distribution=distribution, distroseries=distroseries,
307 architecture=architecture, owner=owner,
308 created_before=created_before, created_after=created_after,
309 submitted_before=submitted_before,
310 submitted_after=submitted_after)
311
299 def getDistroTarget(self, distribution, distroseries, distroarchseries):312 def getDistroTarget(self, distribution, distroseries, distroarchseries):
300 distro_targets = [313 distro_targets = [
301 target for target in (314 target for target in (
302315
=== modified file 'lib/lp/hardwaredb/interfaces/hwdb.py'
--- lib/lp/hardwaredb/interfaces/hwdb.py 2011-02-24 15:30:54 +0000
+++ lib/lp/hardwaredb/interfaces/hwdb.py 2011-06-16 13:11:48 +0000
@@ -283,7 +283,9 @@
283 """283 """
284284
285 def search(user=None, device=None, driver=None, distribution=None,285 def search(user=None, device=None, driver=None, distribution=None,
286 distroseries=None, architecture=None, owner=None):286 distroseries=None, architecture=None, owner=None,
287 created_before=None, created_after=None,
288 submitted_before=None, submitted_after=None):
287 """Return the submissions matiching the given parmeters.289 """Return the submissions matiching the given parmeters.
288290
289 :param user: The `IPerson` running the query. Private submissions291 :param user: The `IPerson` running the query. Private submissions
@@ -300,6 +302,14 @@
300 :param architecture: Limit results to submissions made for302 :param architecture: Limit results to submissions made for
301 a specific architecture.303 a specific architecture.
302 :param owner: Limit results to submissions from this person.304 :param owner: Limit results to submissions from this person.
305 :param created_before: Exclude results created after this
306 date.
307 :param created_after: Exclude results created before or on
308 this date.
309 :param submitted_before: Exclude results submitted after this
310 date.
311 :param submitted_after: Exclude results submitted before or on
312 this date.
303313
304 Only one of :distribution: or :distroseries: may be supplied.314 Only one of :distribution: or :distroseries: may be supplied.
305 """315 """
@@ -1235,6 +1245,105 @@
1235 readonly=True))1245 readonly=True))
12361246
1237 @operation_parameters(1247 @operation_parameters(
1248 device=Reference(
1249 IHWDevice,
1250 title=u'A Device',
1251 description=(
1252 u'If specified, the result set is limited to submissions '
1253 u'containing this device.'),
1254 required=False),
1255 driver=Reference(
1256 IHWDriver,
1257 title=u'A Driver',
1258 description=(
1259 u'If specified, the result set is limited to submissions '
1260 u'containing devices that use this driver.'),
1261 required=False),
1262 distribution=Reference(
1263 IDistribution,
1264 title=u'A Distribution',
1265 description=(
1266 u'If specified, the result set is limited to submissions '
1267 u'made for this distribution.'),
1268 required=False),
1269 distroseries=Reference(
1270 IDistroSeries,
1271 title=u'A Distribution Series',
1272 description=(
1273 u'If specified, the result set is limited to submissions '
1274 u'made for the given distribution series.'),
1275 required=False),
1276 architecture=TextLine(
1277 title=u'A processor architecture',
1278 description=
1279 u'If specified, the result set is limited to sumbissions '
1280 'made for a specific architecture.',
1281 required=False),
1282 owner=Reference(
1283 IPerson,
1284 title=u'Person',
1285 description=
1286 u'If specified, the result set is limited to sumbissions '
1287 'from this person.',
1288 required=False),
1289 created_before=Datetime(
1290 title=u'Created Before',
1291 description=
1292 u'If specified, exclude results created after this date.',
1293 required=False),
1294 created_after=Datetime(
1295 title=u'Created After',
1296 description=
1297 u'If specified, exclude results created before or on '
1298 'this date.',
1299 required=False),
1300 submitted_before=Datetime(
1301 title=u'Created Before',
1302 description=
1303 u'If specified, exclude results submitted after this date.',
1304 required=False),
1305 submitted_after=Datetime(
1306 title=u'Created After',
1307 description=
1308 u'If specified, Exclude results submitted before or on '
1309 'this date.',
1310 required=False))
1311 @call_with(user=REQUEST_USER)
1312 @operation_returns_collection_of(IHWSubmission)
1313 @export_read_operation()
1314 def search(user=None, device=None, driver=None, distribution=None,
1315 distroseries=None, architecture=None, owner=None,
1316 created_before=None, created_after=None,
1317 submitted_before=None, submitted_after=None):
1318 """Return the submissions matiching the given parmeters.
1319
1320 :param user: The `IPerson` running the query. Private submissions
1321 are returned only if the person running the query is the
1322 owner or an admin.
1323 :param device: Limit results to submissions containing this
1324 `IHWDevice`.
1325 :param driver: Limit results to submissions containing devices
1326 that use this `IHWDriver`.
1327 :param distribution: Limit results to submissions made for
1328 this `IDistribution`.
1329 :param distroseries: Limit results to submissions made for
1330 this `IDistroSeries`.
1331 :param architecture: Limit results to submissions made for
1332 a specific architecture.
1333 :param owner: Limit results to submissions from this person.
1334 :param created_before: Exclude results created after this
1335 date.
1336 :param created_after: Exclude results created before or on
1337 this date.
1338 :param submitted_before: Exclude results submitted after this
1339 date.
1340 :param submitted_after: Exclude results submitted before or on
1341 this date.
1342
1343 Only one of :distribution: or :distroseries: may be supplied.
1344 """
1345
1346 @operation_parameters(
1238 bus=Choice(1347 bus=Choice(
1239 title=u'The device bus', vocabulary=HWBus, required=False),1348 title=u'The device bus', vocabulary=HWBus, required=False),
1240 vendor_id=TextLine(1349 vendor_id=TextLine(
12411350
=== modified file 'lib/lp/hardwaredb/model/hwdb.py'
--- lib/lp/hardwaredb/model/hwdb.py 2011-05-27 21:12:25 +0000
+++ lib/lp/hardwaredb/model/hwdb.py 2011-06-16 13:11:48 +0000
@@ -294,7 +294,9 @@
294 return result_set294 return result_set
295295
296 def search(self, user=None, device=None, driver=None, distribution=None,296 def search(self, user=None, device=None, driver=None, distribution=None,
297 distroseries=None, architecture=None, owner=None):297 distroseries=None, architecture=None, owner=None,
298 created_before=None, created_after=None,
299 submitted_before=None, submitted_after=None):
298 """See `IHWSubmissionSet`."""300 """See `IHWSubmissionSet`."""
299 store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)301 store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
300 args = []302 args = []
@@ -330,6 +332,14 @@
330 args.append(DistroArchSeries.distroseries == distroseries.id)332 args.append(DistroArchSeries.distroseries == distroseries.id)
331 if owner is not None:333 if owner is not None:
332 args.append(HWSubmission.owner == owner.id)334 args.append(HWSubmission.owner == owner.id)
335 if created_before is not None:
336 args.append(HWSubmission.date_created <= created_before)
337 if created_after is not None:
338 args.append(HWSubmission.date_created > created_after)
339 if submitted_before is not None:
340 args.append(HWSubmission.date_submitted <= submitted_before)
341 if submitted_after is not None:
342 args.append(HWSubmission.date_submitted > submitted_after)
333343
334 result_set = store.find(344 result_set = store.find(
335 HWSubmission,345 HWSubmission,
336346
=== modified file 'lib/lp/hardwaredb/stories/webservice/xx-hwdb.txt'
--- lib/lp/hardwaredb/stories/webservice/xx-hwdb.txt 2011-02-13 22:54:48 +0000
+++ lib/lp/hardwaredb/stories/webservice/xx-hwdb.txt 2011-06-16 13:11:48 +0000
@@ -711,6 +711,100 @@
711 ...711 ...
712712
713713
714=== Searching for submissions ===
715
716Alternatively, we can also search for hardware submissions by user:
717
718 >>> owner = webservice.getAbsoluteUrl('/~name12')
719 >>> submissions = webservice.get(
720 ... '/+hwdb?ws.op=search&owner=%s' % owner).jsonBody()
721 >>> print submissions['total_size']
722 2
723
724 >>> owner = webservice.getAbsoluteUrl('/~name20')
725 >>> submissions = webservice.get(
726 ... '/+hwdb?ws.op=search&owner=%s' % owner).jsonBody()
727 >>> print submissions['total_size']
728 0
729
730...and by device:
731
732 >>> device = webservice.getAbsoluteUrl('/+hwdb/+device/1')
733 >>> submissions = webservice.named_get(
734 ... '/+hwdb', 'search', device=device).jsonBody()
735 >>> print submissions['total_size']
736 1
737
738...and by driver:
739
740 >>> driver = webservice.getAbsoluteUrl('/+hwdb/+driver/1')
741 >>> submissions = webservice.named_get(
742 ... '/+hwdb', 'search', driver=driver).jsonBody()
743 >>> print submissions['total_size']
744 1
745
746...and by distribution:
747
748 >>> ubuntu_url = webservice.getAbsoluteUrl('/ubuntu')
749 >>> submissions = webservice.named_get(
750 ... '/+hwdb', 'search', distribution=ubuntu_url).jsonBody()
751 >>> print submissions['total_size']
752 1
753 >>> debian_url = webservice.getAbsoluteUrl('/debian')
754 >>> submissions = webservice.named_get(
755 ... '/+hwdb', 'search', distribution=debian_url).jsonBody()
756 >>> print submissions['total_size']
757 0
758
759...and by distroseries:
760
761 >>> hoary_url = webservice.getAbsoluteUrl('/ubuntu/hoary')
762 >>> submissions = webservice.named_get(
763 ... '/+hwdb', 'search', distroseries=hoary_url).jsonBody()
764 >>> print submissions['total_size']
765 1
766 >>> warty_url = webservice.getAbsoluteUrl('/ubuntu/warty')
767 >>> submissions = webservice.named_get(
768 ... '/+hwdb', 'search', distroseries=warty_url).jsonBody()
769 >>> print submissions['total_size']
770 0
771
772...and by architecture:
773
774 >>> submissions = webservice.named_get(
775 ... '/+hwdb', 'search', architecture='i386').jsonBody()
776 >>> print submissions['total_size']
777 1
778 >>> submissions = webservice.named_get(
779 ... '/+hwdb', 'search', architecture='powerpc').jsonBody()
780 >>> print submissions['total_size']
781 0
782
783...and by date created:
784
785 >>> date_created = u'2007-09-11T00:00:00+00:00'
786 >>> submissions = webservice.named_get(
787 ... '/+hwdb', 'search', created_before=date_created).jsonBody()
788 >>> print submissions['total_size']
789 1
790 >>> submissions = webservice.named_get(
791 ... '/+hwdb', 'search', created_after=date_created).jsonBody()
792 >>> print submissions['total_size']
793 1
794
795...and by date submitted:
796
797 >>> date_submitted = u'2007-09-11T15:23:45.653316+00:00'
798 >>> submissions = webservice.named_get(
799 ... '/+hwdb', 'search', submitted_before=date_submitted).jsonBody()
800 >>> print submissions['total_size']
801 1
802 >>> submissions = webservice.named_get(
803 ... '/+hwdb', 'search', submitted_after=date_submitted).jsonBody()
804 >>> print submissions['total_size']
805 1
806
807
714=== Submission Devices ===808=== Submission Devices ===
715809
716The table HWSubmissionDevice associates devices with submissions.810The table HWSubmissionDevice associates devices with submissions.

Subscribers

People subscribed via source and target branches

to status/vote changes: