Merge lp:~cr3/launchpad/hwsubmissionset_search into lp:launchpad

[Marc Tardif]

Contributing changes to expose IHWSubmissionSet.search in the API and extend it to support searching HWSubmissions by date created and date submitted. Note that the method is only available in the beta API and only accessible to people under ~hwdb-team to be consistent with the other methods in the interface.

[Jeroen T. Vermeulen (jtv)]

Hi Marc,

Thanks for working on this — as a development team we often don't feel the pain from needing more API as acutely, though we are rationally aware that it exists. So it's great to see people "scratching their own itch."

I'm voting Resubmit on this because of an annoying technicality. The branch includes a schema patch, which means that the branch needs to be based on db-devel (not devel, which is what shows up as "launchpad" in a branch link) and proposed for merging into db-devel. This isn't very obvious at the moment because we just merged the two for our monthly release, but soon we'll start seeing unrelated changes show up in the diff as the two diverge and everything gets very confusing.

I didn't try to fix this for you, because having two captains on one ship is not a good way to get through a storm. Frankly I usually get out of this situation myself by getting a diff from bzr, cleaning unrelated changes out of it, and applying it to a fresh branch.

Once the merge proposal is in good working order, we proceed as follows:
 * You request a code review (like you did here). Feel free to ping me or whoever is listed as the on-call reviewer in the #launchpad-dev topic at the time.
 * You request two reviews, of type "db," from Stuart Bishop and Robert Collins respectively.
 * Stub assigns you a patch number, which you substitute for the "99" in the patch (both in the filename and in the revision table update inside the patch).

You're good to land once the patch is updated, you have an Approve vote from the code review, and you have _one_ Approve vote from the two db reviews. (If there are any disapprovals, those will have to be dealt with as well of course).

Since you probably don't have landing permission on Launchpad, you'll have to ask us to do it for you. Typically your code reviewer is a good person to handle that, but *feel free to nag* and agree on who will do it in advance if possible. In our process it's normally the author's responsibility to make sure that a branch gets landed, so we don't always remember to look for other people's approved branches that need landing. Nobody on the team will be offended if you ask them to help out with this.

Jeroen

review: Needs Resubmitting (code)

[Jeroen T. Vermeulen (jtv)]

Oh, and separately from that: the copyright notice on the patch says 2009. Probably not what you meant. :-)

[Marc Tardif (cr3)]

Regarding the technicality, I misunderstood one of the steps described in the DatabaseSchemaChangesProcess wiki page:

  4. If your branch also contains code changes, then request a separate code review as well.

This was the purpose of this code review whereas the step you refer to is the previous one in the wiki page:

  3. Submit a merge proposal for your branch into lp:launchpad/db-devel...

This was done separately in the following code review, I thought I needed to separate reviews:

  https://code.launchpad.net/~cr3/launchpad/hwsubmissionset_search/+merge/63768

Stub has approved the code review and provided a patch number, I updated the branch with the necessary changes and he then requested a code review from the Canonical Launchpad Engineering team. So, if I understand correctly his actions and your review comment, I now just need an Approve vote for the code review and I should be good to land.

The one thing that is still not clear is when to submit the code for review in lp:launchpad/devel after it is approved in lp:launchpad/db-devel. Is this done after getting an Approve vote from the Canonical Launchpad Engineering team on the latter branch? If so, is it the reviewer or the requester that then submits to the former branch?

[Robert Collins (lifeless)]

Hi marc, basically your other merge proposal is all you need; thats reviewed and going through the process, so I'm going to delete this version of the proposal.

[Robert Collins (lifeless)]

(actually I've rejected the *proposal* so the comments are kept, but see the other proposal for the ongoing review).

=== added file 'database/schema/patch-2208-74-0.sql'
--- database/schema/patch-2208-74-0.sql	1970-01-01 00:00:00 +0000
+++ database/schema/patch-2208-74-0.sql	2011-06-08 12:41:54 +0000
@@ -0,0 +1,12 @@
+-- Copyright 2011 Canonical Ltd.  This software is licensed under the
+-- GNU Affero General Public License version 3 (see the file LICENSE).
+
+SET client_min_messages=ERROR;
+
+-- For IHWSubmissionSet, which can now search by date_created.
+CREATE INDEX hwsubmission__date_created__idx ON hwsubmission USING btree (date_created);
+
+-- For IHWSubmissionSet, which can now search by date_submitted.
+CREATE INDEX hwsubmission__date_submitted__idx ON hwsubmission USING btree (date_submitted);
+
+INSERT INTO LaunchpadDatabaseRevision VALUES (2208, 74, 0);

=== modified file 'lib/canonical/launchpad/interfaces/_schema_circular_imports.py'
--- lib/canonical/launchpad/interfaces/_schema_circular_imports.py	2011-06-07 05:02:53 +0000
+++ lib/canonical/launchpad/interfaces/_schema_circular_imports.py	2011-06-08 12:41:54 +0000
@@ -879,7 +879,7 @@
 patch_operations_explicit_version(
     IHWDBApplication, 'beta', "deviceDriverOwnersAffectedByBugs", "devices",
     "drivers", "hwInfoByBugRelatedUsers", "numDevicesInSubmissions",
-    "numOwnersOfDevice", "numSubmissionsWithDevice", "vendorIDs")
+    "numOwnersOfDevice", "numSubmissionsWithDevice", "search", "vendorIDs")
 
 # IHWDevice
 patch_entry_explicit_version(IHWDevice, 'beta')

=== 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-08 12:41:54 +0000
@@ -296,6 +296,19 @@
         """See `IHWDBApplication`."""
         return getUtility(IHWDriverSet).all_package_names()
 
+    def search(self, user=None, device=None, driver=None, distribution=None,
+               distroseries=None, architecture=None, owner=None,
+               created_before=None, created_after=None,
+               submitted_before=None, submitted_after=None):
+        """See `IHWDBApplication`."""
+        return getUtility(IHWSubmissionSet).search(
+            user=user, device=device, driver=driver,
+            distribution=distribution, distroseries=distroseries,
+            architecture=architecture, owner=owner,
+            created_before=created_before, created_after=created_after,
+            submitted_before=submitted_before,
+            submitted_after=submitted_after)
+
     def getDistroTarget(self, distribution, distroseries, distroarchseries):
         distro_targets = [
             target for target in (

=== 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-08 12:41:54 +0000
@@ -283,7 +283,9 @@
         """
 
     def search(user=None, device=None, driver=None, distribution=None,
-               distroseries=None, architecture=None, owner=None):
+               distroseries=None, architecture=None, owner=None,
+               created_before=None, created_after=None,
+               submitted_before=None, submitted_after=None):
         """Return the submissions matiching the given parmeters.
 
         :param user: The `IPerson` running the query. Private submissions
@@ -300,6 +302,14 @@
         :param architecture: Limit results to submissions made for
             a specific architecture.
         :param owner: Limit results to submissions from this person.
+        :param created_before: Limit results to submissions created
+            before this date inclusively.
+        :param created_after: Limit results to submissions created
+            after this date exclusively.
+        :param submitted_before: Limit results to submissions submitted
+            before this date inclusively.
+        :param submitted_after: Limit results to submissions submitted
+            after this date exclusively.
 
         Only one of :distribution: or :distroseries: may be supplied.
         """
@@ -1235,6 +1245,107 @@
             readonly=True))
 
     @operation_parameters(
+        device=Reference(
+            IHWDevice,
+            title=u'A Device',
+            description=(
+                u'If specified, the result set is limited to submissions '
+                u'containing this device.'),
+            required=False),
+        driver=Reference(
+            IHWDriver,
+            title=u'A Driver',
+            description=(
+                u'If specified, the result set is limited to submissions '
+                u'containing devices that use this driver.'),
+            required=False),
+        distribution=Reference(
+            IDistribution,
+            title=u'A Distribution',
+            description=(
+                u'If specified, the result set is limited to submissions '
+                u'made for this distribution.'),
+            required=False),
+        distroseries=Reference(
+            IDistroSeries,
+            title=u'A Distribution Series',
+            description=(
+                u'If specified, the result set is limited to submissions '
+                u'made for the given distribution series.'),
+            required=False),
+        architecture=TextLine(
+            title=u'A processor architecture',
+            description=
+                u'If specified, the result set is limited to sumbissions '
+                'made for a specific architecture.',
+            required=False),
+        owner=Reference(
+            IPerson,
+            title=u'Person',
+            description=
+                u'If specified, the result set is limited to sumbissions '
+                'from this person.',
+            required=False),
+        created_before=Datetime(
+            title=u'Created Before',
+            description=
+                u'If specified, the result set is limited to submissions '
+                'created before this date inclusively',
+            required=False),
+        created_after=Datetime(
+            title=u'Created After',
+            description=
+                u'If specified, the result set is limited to submissions '
+                'created after this date exclusively',
+            required=False),
+        submitted_before=Datetime(
+            title=u'Created Before',
+            description=
+                u'If specified, the result set is limited to submissions '
+                'submitted before this date inclusively',
+            required=False),
+        submitted_after=Datetime(
+            title=u'Created After',
+            description=
+                u'If specified, the result set is limited to submissions '
+                'submitted after this date exclusively',
+            required=False))
+    @call_with(user=REQUEST_USER)
+    @operation_returns_collection_of(IHWSubmission)
+    @export_read_operation()
+    def search(user=None, device=None, driver=None, distribution=None,
+               distroseries=None, architecture=None, owner=None,
+               created_before=None, created_after=None,
+               submitted_before=None, submitted_after=None):
+        """Return the submissions matiching the given parmeters.
+
+        :param user: The `IPerson` running the query. Private submissions
+            are returned only if the person running the query is the
+            owner or an admin.
+        :param device: Limit results to submissions containing this
+            `IHWDevice`.
+        :param driver: Limit results to submissions containing devices
+            that use this `IHWDriver`.
+        :param distribution: Limit results to submissions made for
+            this `IDistribution`.
+        :param distroseries: Limit results to submissions made for
+            this `IDistroSeries`.
+        :param architecture: Limit results to submissions made for
+            a specific architecture.
+        :param owner: Limit results to submissions from this person.
+        :param created_before: Limit results to submissions created
+            before this date inclusively.
+        :param created_after: Limit results to submissions created
+            after this date exclusively.
+        :param submitted_before: Limit results to submissions submitted
+            before this date inclusively.
+        :param submitted_after: Limit results to submissions submitted
+            after this date exclusively.
+
+        Only one of :distribution: or :distroseries: may be supplied.
+        """
+
+    @operation_parameters(
         bus=Choice(
             title=u'The device bus', vocabulary=HWBus, required=False),
         vendor_id=TextLine(

=== 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-08 12:41:54 +0000
@@ -294,7 +294,9 @@
         return result_set
 
     def search(self, user=None, device=None, driver=None, distribution=None,
-               distroseries=None, architecture=None, owner=None):
+               distroseries=None, architecture=None, owner=None,
+               created_before=None, created_after=None,
+               submitted_before=None, submitted_after=None):
         """See `IHWSubmissionSet`."""
         store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
         args = []
@@ -328,9 +330,22 @@
                 args.append(Distribution.id == distribution.id)
             if distroseries is not None:
                 args.append(DistroArchSeries.distroseries == distroseries.id)
+
         if owner is not None:
             args.append(HWSubmission.owner == owner.id)
 
+        if created_before is not None:
+            args.append(HWSubmission.date_created <= created_before)
+
+        if created_after is not None:
+            args.append(HWSubmission.date_created > created_after)
+
+        if submitted_before is not None:
+            args.append(HWSubmission.date_submitted <= submitted_before)
+
+        if submitted_after is not None:
+            args.append(HWSubmission.date_submitted > submitted_after)
+
         result_set = store.find(
             HWSubmission,
             _userCanAccessSubmissionStormClause(user),

=== 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-08 12:41:54 +0000
@@ -711,6 +711,100 @@
     ...
 
 
+=== Searching for submissions ===
+
+Alternatively, we can also search for hardware submissions by user:
+
+    >>> owner = webservice.getAbsoluteUrl('/~name12')
+    >>> submissions = webservice.get(
+    ...     '/+hwdb?ws.op=search&owner=%s' % owner).jsonBody()
+    >>> print submissions['total_size']
+    2
+
+    >>> owner = webservice.getAbsoluteUrl('/~name20')
+    >>> submissions = webservice.get(
+    ...     '/+hwdb?ws.op=search&owner=%s' % owner).jsonBody()
+    >>> print submissions['total_size']
+    0
+
+...and by device:
+
+    >>> device = webservice.getAbsoluteUrl('/+hwdb/+device/1')
+    >>> submissions = webservice.named_get(
+    ...     '/+hwdb', 'search', device=device).jsonBody()
+    >>> print submissions['total_size']
+    1
+
+...and by driver:
+
+    >>> driver = webservice.getAbsoluteUrl('/+hwdb/+driver/1')
+    >>> submissions = webservice.named_get(
+    ...     '/+hwdb', 'search', driver=driver).jsonBody()
+    >>> print submissions['total_size']
+    1
+
+...and by distribution:
+
+    >>> ubuntu_url = webservice.getAbsoluteUrl('/ubuntu')
+    >>> submissions = webservice.named_get(
+    ...     '/+hwdb', 'search', distribution=ubuntu_url).jsonBody()
+    >>> print submissions['total_size']
+    1
+    >>> debian_url = webservice.getAbsoluteUrl('/debian')
+    >>> submissions = webservice.named_get(
+    ...     '/+hwdb', 'search', distribution=debian_url).jsonBody()
+    >>> print submissions['total_size']
+    0
+
+...and by distroseries:
+
+    >>> hoary_url = webservice.getAbsoluteUrl('/ubuntu/hoary')
+    >>> submissions = webservice.named_get(
+    ...     '/+hwdb', 'search', distroseries=hoary_url).jsonBody()
+    >>> print submissions['total_size']
+    1
+    >>> warty_url = webservice.getAbsoluteUrl('/ubuntu/warty')
+    >>> submissions = webservice.named_get(
+    ...     '/+hwdb', 'search', distroseries=warty_url).jsonBody()
+    >>> print submissions['total_size']
+    0
+
+...and by architecture:
+
+    >>> submissions = webservice.named_get(
+    ...     '/+hwdb', 'search', architecture='i386').jsonBody()
+    >>> print submissions['total_size']
+    1
+    >>> submissions = webservice.named_get(
+    ...     '/+hwdb', 'search', architecture='powerpc').jsonBody()
+    >>> print submissions['total_size']
+    0
+
+...and by date created:
+
+    >>> date_created = u'2007-09-11T00:00:00+00:00'
+    >>> submissions = webservice.named_get(
+    ...     '/+hwdb', 'search', created_before=date_created).jsonBody()
+    >>> print submissions['total_size']
+    1
+    >>> submissions = webservice.named_get(
+    ...     '/+hwdb', 'search', created_after=date_created).jsonBody()
+    >>> print submissions['total_size']
+    1
+
+...and by date submitted:
+
+    >>> date_submitted = u'2007-09-11T15:23:45.653316+00:00'
+    >>> submissions = webservice.named_get(
+    ...     '/+hwdb', 'search', submitted_before=date_submitted).jsonBody()
+    >>> print submissions['total_size']
+    1
+    >>> submissions = webservice.named_get(
+    ...     '/+hwdb', 'search', submitted_after=date_submitted).jsonBody()
+    >>> print submissions['total_size']
+    1
+
+
 === Submission Devices ===
 
 The table HWSubmissionDevice associates devices with submissions.