launchpad-reviewers team mailing list archive

[Curtis Hovey <curtis.hovey@xxxxxxxxxxxxx>]

Curtis Hovey has proposed merging lp:~sinzui/launchpad/dsp-db-assertion-0 into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #611274 distributionsourcepackage not created before job
  https://bugs.launchpad.net/bugs/611274


This is my branch to remove the assertion for dsp in db.

    lp:~sinzui/launchpad/dsp-db-assertion-0
    Diff size: 27
    Launchpad bug: https://bugs.launchpad.net/bugs/611274
    Pre-implementation: Edwin
    Target release: 10.10


Remove the assertion for dsp in db
----------------------------------

I can see that most, maybe all, lpnet versions of the oops are for distros
other than Ubuntu--this is not an error. Launchpad do not need db backing for
these because we know the SPNs can be bogus and there never will be publishing
history. BugWatch is the most common reason for this

I can see that most of the edge version of the oops are for Ubuntu. In the
gio-sharp example, the first ever bug was reported on the package. it was
linked to a project two weeks ago by Jelmer. Bug 629899 was reported for the
DSP on 2010-09-03, and that caused a call to DSP.recalculateBugHeatCache().
This is the correct operation. No error for the user and the moment new data
was provided launchpad created the db instance. We might think that the DB
instance could have been created earlier such as for translatable messages,
but gio-sharp has no translatable messages.

After much discussion, it was decided that the assert code is not really
an error. The information we learned is very helpful in that it demonstrated
that there are several cases where we do not want to, or do not need to,
create a db instance of a DSP until the very moment we decided to save
data about it. Lp is creating the DB instance on demand. Since there is
no other way for the callsite to create the DSP in db, there the callsite
is not in error.

We may want to ensure every Ubuntu DSP has a db instance for reporting
purposes, but the current reporting purposes does not require it. If we
decide to require it, we will consider merging the DSP search cache table
with the DSP table to ensure every DSP that can be search also has statistical
data and bug reporting guide lines too.


Rules
-----

    * Remove the code that quietly raises an oops.


QA
--

    * Watch the oopses in edge to verify that bug reporting does not causes
      the oops.


Lint
----

Linting changed files:
  lib/lp/registry/model/distributionsourcepackage.py


Test
----

No tests changed.


Implementation
--------------

    * lib/lp/registry/model/distributionsourcepackage.py
      * Removed the code the quietly raises an oops.
-- 
https://code.launchpad.net/~sinzui/launchpad/dsp-db-assertion-0/+merge/35232
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/dsp-db-assertion-0 into lp:launchpad/devel.

=== modified file 'lib/lp/registry/model/distributionsourcepackage.py'
--- lib/lp/registry/model/distributionsourcepackage.py	2010-08-26 08:02:08 +0000
+++ lib/lp/registry/model/distributionsourcepackage.py	2010-09-12 17:51:06 +0000
@@ -34,8 +34,6 @@
     Unicode,
     )
 from storm.store import EmptyResultSet
-from zope.component import getUtility
-from zope.error.interfaces import IErrorReportingUtility
 from zope.interface import implements
 
 from canonical.database.sqlbase import sqlvalues
@@ -113,13 +111,6 @@
 
     def __set__(self, obj, value):
         if obj._self_in_database is None:
-            # Log an oops without raising an error.
-            exception = AssertionError(
-                "DistributionSourcePackage record should have been created "
-                "earlier in the database for distro=%s, sourcepackagename=%s"
-                % (obj.distribution.name, obj.sourcepackagename.name))
-            getUtility(IErrorReportingUtility).raising(
-                (exception.__class__, exception, None))
             spph = Store.of(obj.distribution).find(
                 SourcePackagePublishingHistory,
                 SourcePackagePublishingHistory.distroseriesID ==