Merge lp:~jtv/launchpad/cp2-bug-403992 into lp:launchpad

[Jeroen T. Vermeulen]

[Jeroen T. Vermeulen (jtv)]

This is a rerun of the merge request for lp:~jtv/launchpad/cp-bug-403992
which accidentally got other branches in its diff.

= Bug 403992 =

I'm hoping to get this branch cherry-picked.

Message-sharing migration is failing. This is the command that broke on
us:

    ./scripts/rosetta/message-sharing-merge.py -vvv -P elisa

The problem is with some caching the script does. It keeps a cache of
TranslationMessages for the representative POTMsgSet it's merging
subordinate POTMsgSets into. It uses this cache to detect when moving
a current or imported TranslationMessage from a subordinate POTMsgSet to
the representative POTMsgSet would conflict with an existing current or
imported TranslationMessage.

Unfortunately the script fails to update this cache with the additions
to the representative POTMsgSet as it moves TranslationMessages over.
Thus a conflict could go undetected and violate a unique constraint on
the database.

This branch fixes the problem minimally by using the non-caching method
to detect clashes, rather than the broken cache. A separate branch,
lp:~jtv/launchpad/bug-403992 also culls the dead code and its unit
tests.

There are no test changes here. Basic functionality is retained, and I
see no easy way for this exact problem to come back now that the cache
is no longer in use.

Jeroen

[Celso Providelo (cprov)]

Jeroen,

First, thanks for providing a fix for this production issue.

From the code-review PoV it's fine. I take it as 'just removing some premature-optimization that was broken'. You say it doesn't have any major performance implication.

Although, I'm concerned about the cherrypicking it in production without a clear indication that it is solving the problem, not just dodging the OOPS. Lack of tests associated with superficial knowledge about the code is always scary.

Anyway, r=me for the clashing-detection method swap and let's see what the kiko/francis says about the CP request.

review: Approve

=== modified file 'lib/lp/translations/scripts/message_sharing_migration.py'
--- lib/lp/translations/scripts/message_sharing_migration.py	2009-07-19 04:41:14 +0000
+++ lib/lp/translations/scripts/message_sharing_migration.py	2009-08-06 13:16:15 +0000
@@ -309,8 +309,8 @@
                     message = removeSecurityProxy(message)
 
                     clashing_current, clashing_imported, twin = (
-                        self._findClashesFromDicts(
-                            existing_tms, current_tms, imported_tms, message))
+                        self._findClashes(
+                            message, representative, message.potemplate))
 
                     if clashing_current or clashing_imported:
                         saved = self._saveByDiverging(