launchpad-reviewers team mailing list archive

[Curtis Hovey <curtis.hovey@xxxxxxxxxxxxx>]

Curtis Hovey has proposed merging lp:~sinzui/launchpad/missing-potmsgset into lp:launchpad.

Commit message:
Ensure POTMsgSets are in the db during poimport.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #879356 in Launchpad itself: "Foreign-key constraint violation in translations importer"
  https://bugs.launchpad.net/launchpad/+bug/879356

For more details, see:
https://code.launchpad.net/~sinzui/launchpad/missing-potmsgset/+merge/134483

The rosetta-poimport script is violating referential integrity:

2011-10-20 23:16:23 ERROR insert or update on table "translationmessage"
    violates foreign key constraint "translationmessage__potmsgset__fk"
    DETAIL: Key (potmsgset)=(11731721) is not present in table "potmsgset".
    -- Slovenian (sl) translation of kaddressbook in
       Ubuntu Oneiric package "kdepim" (id 5905106)

Full traceback: http://paste.ubuntu.com/715011/

A query some 12 hours after this particular failure shows that the
highest used id in potmsgset is 11787759. So the problematic id is quite
a new one.

--------------------------------------------------------------------

RULES

    Pre-implementation: jtv, wgrant, wallyworld
    * We do not know for certain why the POTMsgSet is not yet in the DB.
    * We have confirmed that the UnusedPOTMsgSetPruner is interacting
      with poimport, but getOrCreatePOTMsgSet() ensures one will
      exist (in code) when it is needed.
      * Both processes are iterating over a collection and committing at
        each loop. They do change the DB as both run.
      * We Updated UnusedPOTMsgSetPruner to not delete PotMsgSets during
        the loop if it suddenly became used.
    * In 13 lines of code and a few milliseconds pass between the point
      where getOrCreatePOTMsgSet is called and the call to
      POTMsgSet.getCurrentTranslation() causes a flush() to ensure the
      impending query will work. But the oops shows that the POTMsgSet
      did not arrive in time. Within those 13 lines of code, a
      TranslationMessage is created, approved, then set as the
      current translation. The OOPS is actually about the
      act of creating the TranslationMessage, but we do not see this
      DB error until we execute more lines of code working with it and
      its POTMsgSet and the ORM decides to flush.
    * There are rules to flush the top-level objects (POFile and
      POTemplate) after they are created. There are also rules to
      alter the flush order of messages to ensure the act of reassignment
      does not cause a unique key violation. But the intermediate
      POTMsgSet is not explicitly flushed.
    * Else where in Lp the POTMsgSet is explicitly flushed when it is
      created or modified. In fact the same list of modified POTMsgSet
      attrs get flushed by the testing factory as is changed by poimport.
    * There are two places to call flush() to ensure the POTMsgSet is in
      the DB. 1, immediately after the change to the attrs as is done
      else where, or 2, in storeTranslationsInDatabase()
      * The call to storeTranslationsInDatabase() is exactly where the
        call to flush would be added, so the intent of the method is
        to get all the data into the DB -- Add a call to flush() in
        storeTranslationsInDatabase() one we are certain there wont be
        an early exit.


QA

    * None. We do not know how to create the issue. This code just ensures
      the case cannot happen.

LoC

    * I have a 3000 line credit this week.


LINT

    lib/lp/translations/utilities/translation_import.py


TEST

    ./bin/test -vv lp.translations.utilities.tests.test_file_importer


IMPLEMENTATION

Added a call to flush() the ORM when it is certain the data needs to go
to the DB. Cleanup lint.
    lib/lp/translations/utilities/translation_import.py
-- 
https://code.launchpad.net/~sinzui/launchpad/missing-potmsgset/+merge/134483
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/missing-potmsgset into lp:launchpad.

=== modified file 'lib/lp/translations/utilities/translation_import.py'
--- lib/lp/translations/utilities/translation_import.py	2012-06-29 08:40:05 +0000
+++ lib/lp/translations/utilities/translation_import.py	2012-11-15 15:18:30 +0000
@@ -25,6 +25,7 @@
     )
 from lp.registry.interfaces.sourcepackage import ISourcePackageFactory
 from lp.services.config import config
+from lp.services.database.lpstorm import IStore
 from lp.services.database.sqlbase import (
     cursor,
     quote,
@@ -373,7 +374,7 @@
     """
 
     def __init__(self, translation_import_queue_entry,
-                 importer, logger = None):
+                 importer, logger=None):
         """Base constructor to set up common attributes and parse the imported
         file into a member variable (self.translation_file).
 
@@ -536,6 +537,10 @@
             potmsgset.singular_text, message_data.translations,
             self.pofile.language.pluralforms)
 
+        # Flush the store now because flush order rules can cause messages
+        # to be flushed before the potmsgset arrives in the database.
+        IStore(potmsgset).flush()
+
         if potmsgset.is_translation_credit:
             # Translation credits cannot be added as suggestions.
             return self._storeCredits(potmsgset, sanitized_translations)
@@ -680,7 +685,7 @@
             message._translations = None
 
         if len(message.flags) > 0:
-            flags_comment = u", "+u", ".join(message.flags)
+            flags_comment = u", " + u", ".join(message.flags)
         else:
             flags_comment = u""