Code review comment for lp:~gmb/launchpad/subscribers-timeout-bug-487015

Revision history for this message
Graham Binns (gmb) wrote :

This branch fixes bug 487015 by adding methods to IBug that allow us to
check what kind of subscription (if any) a Person has to a bug using DB
queries rather than getting the list of subscribers and iterating over
it.

I've added three methods:

 - personIsDirectSubscriber(): returns True only if the Person has an
   explicit BugSubscription for the current bug.
 - personIsSubscribedToDuplicate(): returns True only if the Person has
   a BugSubscription to one of a bug's duplicates.
 - personIsAlsoNotifiedSubscriber(): returns True if the Person doesn't
   have a BugSubscription for a bug but will receive bugmail anyway
   (assignees, contacts and structural subscribers fall into this
   category).

Note that personIsAlsoNotifiedSubscriber() and personIsDirectSubscriber()
are mutually exclusive but personIsDirectSubscriber() and
personIsSubscribedToDuplicate() are not.

For personIsDirectSubscriber() and personIsSubscribedToDuplicate() I've
used Storm to create the necessary DB queries. However, I couldn't do
this for personIsAlsoNotifiedSubscriber() because "also notified
subscribers" come from a mish-mash of sources. I don't think this is a
problem (at least not right now) because there are usually less
"also notified" subscribers than there are subscribers from duplicates,
and the timeouts that raised this bug in the first place are due to the
number of duplicate subscribers and the amount of iteration we do over
that set.

I've updated BugViewMixin.subscription_class, which was where the
timeout problem identified in bug 487015 originated, to use the new
methods when determining what CSS class to return for a subscription.
Hopefully this will speed matters up somewhat.

I've also done a drive-by conversion of bug.txt to ReST.

NOTE: `make lint` shows a lot of "Operator is not preceded by a space"
errors, which look like crack to me and I'm not sure what to do about
them. Any ideas?

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/bugs/configure.zcml
  lib/lp/bugs/browser/bug.py
  lib/lp/bugs/doc/bug.txt
  lib/lp/bugs/interfaces/bug.py
  lib/lp/bugs/model/bug.py

== Pylint notices ==

lib/lp/bugs/browser/bug.py
    28: [F0401] Unable to import 'email.MIMEMultipart' (No module named MIMEMultipart)
    29: [F0401] Unable to import 'email.MIMEText' (No module named MIMEText)
    43: [F0401] Unable to import 'lazr.enum' (No module named enum)
    44: [F0401] Unable to import 'lazr.lifecycle.event' (No module named lifecycle)
    45: [F0401] Unable to import 'lazr.lifecycle.snapshot' (No module named lifecycle)
    46: [F0401] Unable to import 'lazr.restful.interfaces' (No module named restful)

lib/lp/bugs/interfaces/bug.py
    49: [F0401] Unable to import 'lazr.restful.declarations' (No module named restful)
    55: [F0401] Unable to import 'lazr.restful.fields' (No module named restful)
    56: [F0401] Unable to import 'lazr.restful.interface' (No module named restful)
    466: [C0322, IBug.addAttachment] Operator not preceded by a space
    comment=Text(), filename=TextLine(), is_patch=Bool(),
    ^
    content_type=TextLine(), description=Text())
    @export_factory_operation(IBugAttachment, [])
    def addAttachment(owner, data, comment, filename, is_patch=False,
    content_type=None, description=None):
    601: [C0322, IBug.getNominations] Operator not preceded by a space
    nominations=List(
    ^
    title=_("Nominations to search through."),
    value_type=Reference(schema=Interface), # IBugNomination
    required=False))
    @operation_returns_collection_of(Interface) # IBugNomination
    @export_read_operation()
    def getNominations(target=None, nominations=None):
    681: [C0322, IBug.markUserAffected] Operator not preceded by a space
    required=False, default=True))
    ^
    @call_with(user=REQUEST_USER)
    @export_write_operation()
    def markUserAffected(user, affected=True):
    696: [C0322, IBug.setCommentVisibility] Operator not preceded by a space
    required=True),
    ^
    visible=Bool(title=_('Show this comment?'), required=True))
    @call_with(user=REQUEST_USER)
    @export_write_operation()
    def setCommentVisibility(user, comment_number, visible):
    708: [C0322, IBug.linkHWSubmission] Operator not preceded by a space
    Interface, title=_('A HWDB submission'), required=True))
    ^
    @export_write_operation()
    def linkHWSubmission(submission):
    715: [C0322, IBug.unlinkHWSubmission] Operator not preceded by a space
    Interface, title=_('A HWDB submission'), required=True))
    ^
    @export_write_operation()
    def unlinkHWSubmission(submission):

lib/lp/bugs/model/bug.py
    25: [F0401] Unable to import 'email.Utils' (No module named Utils)
    38: [F0401] Unable to import 'lazr.lifecycle.event' (No module named lifecycle)
    40: [F0401] Unable to import 'lazr.lifecycle.snapshot' (No module named lifecycle)

« Back to merge proposal