launchpad-dev team mailing list archive

[Muharem Hrnjadovic <muharem@xxxxxxxxxx>]

Bjorn Tillenius wrote:
> On Fri, Oct 23, 2009 at 10:13:14PM -0000, noreply@xxxxxxxxxxxxx wrote:
>> Merge authors:
>>   Muharem Hrnjadovic (al-maisan)
>> Related merge proposals:
>>   https://code.launchpad.net/~al-maisan/launchpad/clog-oops-452070/+merge/13789
>>   proposed by: Muharem Hrnjadovic (al-maisan)
>>   review: Approve - Tom Berger (intellectronica)
>> ------------------------------------------------------------
>> revno: 9762 [merge]
>> committer: Launchpad Patch Queue Manager <launchpad@xxxxxxxxxxxxxxxxx>
>> branch nick: launchpad
>> timestamp: Fri 2009-10-23 23:09:49 +0100
>> message:
>>   [r=intellectronica][ui=none][bug=452070] Optimise
>>   	<distro>/+source/<package>/+changelog pages in order to prevent OOPSs
>> removed:
>>   lib/lp/soyuz/templates/sourcepackagerelease-change-summary.pt
>> modified:
>>   lib/canonical/launchpad/webapp/tales.py
>>   lib/lp/bugs/doc/bug.txt
>>   lib/lp/bugs/interfaces/bug.py
>>   lib/lp/bugs/model/bug.py
>>   lib/lp/registry/browser/distributionsourcepackage.py
>>   lib/lp/registry/configure.zcml
>>   lib/lp/registry/doc/sourcepackage.txt
>>   lib/lp/registry/model/distributionsourcepackage.py
>>   lib/lp/soyuz/browser/configure.zcml
>>   lib/lp/soyuz/browser/sourcepackagerelease.py
>>   lib/lp/soyuz/stories/soyuz/xx-distributionsourcepackagerelease-pages.txt
>>   lib/lp/soyuz/templates/distributionsourcepackagerelease-changes.pt
> 
> 
>> === modified file 'lib/lp/bugs/doc/bug.txt'
>> --- lib/lp/bugs/doc/bug.txt	2009-07-08 03:14:59 +0000
>> +++ lib/lp/bugs/doc/bug.txt	2009-10-22 15:39:19 +0000
>> @@ -37,6 +37,34 @@
>>        ...
>>      NotFoundError: 'Unable to locate bug with nickname +bugs.'
>>  
>> +It is also possible to retrieve a number of bugs by specifying the bug numbers
>> +of interest.
>> +
>> +    >>> result_set = bugset.getByNumbers([6, 1234])
>> +    >>> print result_set.count()
>> +    1

Hello Bjorn,

how are things? Hope you're doing well. Thank you very much for pointing
out improvements to the branch at hand.

> What's a bug number? Don't we usually talk about bug ids?

I touched base with Deryck before adding this method to BugSet and
that's one of the names he proposed that I liked.

> Also, it's highly confusing that you specify two ids/numbers, yet you
> get only one result!
> 
> 
>> +
>> +    >>> [the_bug_found] = result_set
>> +    >>> print the_bug_found.title
>> +    Firefox crashes when Save As dialog for a nonexistent window is closed
>> +
>> +    >>> result_set = bugset.getByNumbers([6, 4321, 1])
>> +    >>> print result_set.count()
>> +    2
> 
> Again, you get one bug less then you asked for. Does it always return
> one result less than you ask for?

Sorry for the misleading test. The changes in the enclosed diff fix this.

>> +
>> +    >>> second_bug_found = result_set[1]
> 
> Where do the results get ordered? Are you guaranteed that you will get
> this bug as the second result?

The getByNumbers() method did not order the returned result set.
The changes in the enclosed diff fix this.

>> +    >>> print second_bug_found.title
>> +    Firefox does not support SVG
> 
> How about showing the actual ids of the bugs returned? It's quite hard
> to associating a title with an id.

Done.

>> === modified file 'lib/lp/bugs/model/bug.py'
>> --- lib/lp/bugs/model/bug.py	2009-08-26 01:54:39 +0000
>> +++ lib/lp/bugs/model/bug.py	2009-10-22 10:33:00 +0000
>> @@ -33,7 +33,7 @@
>>  from sqlobject import SQLMultipleJoin, SQLRelatedJoin
>>  from sqlobject import SQLObjectNotFound
>>  from storm.expr import And, Count, In, LeftJoin, Select, SQLRaw, Func
>> -from storm.store import Store
>> +from storm.store import EmptyResultSet, Store
>>  
>>  from lazr.lifecycle.event import (
>>      ObjectCreatedEvent, ObjectDeletedEvent, ObjectModifiedEvent)
>> @@ -49,6 +49,7 @@
>>  from canonical.launchpad.interfaces.hwdb import IHWSubmissionBugSet
>>  from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
>>  from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet
>> +from canonical.launchpad.interfaces.lpstorm import IStore
>>  from canonical.launchpad.interfaces.message import (
>>      IMessage, IndexedMessage)
>>  from canonical.launchpad.interfaces.structuralsubscription import (
>> @@ -1627,6 +1628,14 @@
>>  
>>          return bugs
>>  
>> +    def getByNumbers(self, bug_numbers):
>> +        """see `IBugSet`."""
>> +        if bug_numbers is None or len(bug_numbers) < 1:
> 
> Can len(bug_numbers) ever be less than 0?
>
>> +            return EmptyResultSet()
>> +        store = IStore(Bug)
>> +        result_set = store.find(Bug, In(Bug.id, bug_numbers))
>> +        return result_set
>> +
>>  
>>  class BugAffectsPerson(SQLBase):
>>      """A bug is marked as affecting a user."""
> 
> 
>> === modified file 'lib/lp/registry/model/distributionsourcepackage.py'
>> --- lib/lp/registry/model/distributionsourcepackage.py	2009-09-16 04:31:39 +0000
>> +++ lib/lp/registry/model/distributionsourcepackage.py	2009-10-22 15:04:42 +0000
>> @@ -408,6 +412,23 @@
>>              'BugTask.distribution = %s AND BugTask.sourcepackagename = %s' %
>>                  sqlvalues(self.distribution, self.sourcepackagename))
>>  
>> +    @staticmethod
>> +    def getPersonsByEmail(email_addresses):
>> +        """[(EmailAddress,Person), ..] iterable for given email addresses."""
>> +        if email_addresses is None or len(email_addresses) < 1:
>> +            return EmptyResultSet()
>> +        # Perform basic sanitization of email addresses.
>> +        email_addresses = [
>> +            address.lower().strip() for address in email_addresses]
>> +        store = IStore(Person)
>> +        origin = [
>> +            Person, Join(EmailAddress, EmailAddress.personID == Person.id)]
>> +        # Get all persons whose email addresses are in the list.
>> +        result_set = store.using(*origin).find(
>> +            (EmailAddress, Person),
>> +            In(Lower(EmailAddress.email), email_addresses))
>> +        return result_set
> 
> Why is getPersonsByEmail a static method on DistributionSourcePackage???
> What does it have to do with source packages? Why is it not in
> PersonSet?

I did email Curtis because that's where the method obviously belongs to.
He did not get back to me (probably too busy due to a sprint) so I left
it where it is with the intention to touch base with him again and
re-factor this code in the process.

> Where are the tests for this method?

I tested this branch on dogfood and made sure that getPersonsByEmail()
works correctly. Besides, the branch was quite big already and I was
anxious not to inflate it further.

The branch that relocates the method in question to PersonSet will have
the tests you requested.

Best regards

-- 
Muharem Hrnjadovic <muharem@xxxxxxxxxx>
Public key id   : B2BBFCFC
Key fingerprint : A5A3 CC67 2B87 D641 103F  5602 219F 6B60 B2BB FCFC

=== modified file 'lib/lp/bugs/doc/bug.txt'
--- lib/lp/bugs/doc/bug.txt	2009-10-22 15:39:19 +0000
+++ lib/lp/bugs/doc/bug.txt	2009-10-26 17:10:37 +0000
@@ -40,6 +40,9 @@
 It is also possible to retrieve a number of bugs by specifying the bug numbers
 of interest.
 
+The method ignores bug numbers not found in the database. That's why the
+result set below has only one element.
+
     >>> result_set = bugset.getByNumbers([6, 1234])
     >>> print result_set.count()
     1
@@ -48,13 +51,13 @@
     >>> print the_bug_found.title
     Firefox crashes when Save As dialog for a nonexistent window is closed
 
-    >>> result_set = bugset.getByNumbers([6, 4321, 1])
+    >>> result_set = bugset.getByNumbers([6, 1])
     >>> print result_set.count()
     2
 
-    >>> second_bug_found = result_set[1]
-    >>> print second_bug_found.title
-    Firefox does not support SVG
+    >>> print [(bug.id, bug.title[:40]) for bug in result_set]
+    [(1, u'Firefox does not support SVG'),
+     (6, u'Firefox crashes when Save As dialog for ')]
 
 If no bug numbers are specified an empty result set is returned.
 

=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2009-10-22 10:33:00 +0000
+++ lib/lp/bugs/model/bug.py	2009-10-26 16:22:36 +0000
@@ -1634,7 +1634,7 @@
             return EmptyResultSet()
         store = IStore(Bug)
         result_set = store.find(Bug, In(Bug.id, bug_numbers))
-        return result_set
+        return result_set.order_by('id')
 
 
 class BugAffectsPerson(SQLBase):

Attachment: signature.asc
Description: OpenPGP digital signature