launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/copyTranslationsFromParent-inline into lp:launchpad.

Commit message:
Inline DistroSeries.copyTranslationsFromParent.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/copyTranslationsFromParent-inline/+merge/231299

Inline DistroSeries.copyTranslationsFromParent. It was only separate to test a couple of assertions that the script ensures anyway.
-- 
https://code.launchpad.net/~wgrant/launchpad/copyTranslationsFromParent-inline/+merge/231299
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/copyTranslationsFromParent-inline into lp:launchpad.

=== modified file 'lib/lp/registry/interfaces/distroseries.py'
--- lib/lp/registry/interfaces/distroseries.py	2014-07-31 00:23:58 +0000
+++ lib/lp/registry/interfaces/distroseries.py	2014-08-19 05:07:37 +0000
@@ -775,20 +775,6 @@
                 supports_virtualized=False, enabled=True):
         """Create a new port or DistroArchSeries for this DistroSeries."""
 
-    def copyTranslationsFromParent(ztm):
-        """Copy any translation done in parent that we lack.
-
-        If there is another translation already added to this one, we ignore
-        the one from parent.
-
-        The supplied transaction manager will be used for intermediate
-        commits to break up large copying jobs into palatable smaller
-        chunks.
-
-        This method starts and commits transactions, so don't rely on `self`
-        or any other database object remaining valid across this call!
-        """
-
     def getPOFileContributorsByLanguage(language):
         """People who translated strings to the given language.
 

=== modified file 'lib/lp/registry/model/distroseries.py'
--- lib/lp/registry/model/distroseries.py	2014-07-31 00:23:58 +0000
+++ lib/lp/registry/model/distroseries.py	2014-08-19 05:07:37 +0000
@@ -14,7 +14,6 @@
 
 import collections
 from cStringIO import StringIO
-import logging
 from operator import attrgetter
 
 import apt_pkg
@@ -106,8 +105,6 @@
 from lp.services.database.enumcol import EnumCol
 from lp.services.database.interfaces import IStore
 from lp.services.database.sqlbase import (
-    flush_database_caches,
-    flush_database_updates,
     SQLBase,
     sqlvalues,
     )
@@ -171,9 +168,6 @@
 from lp.soyuz.model.section import Section
 from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
 from lp.translations.enums import LanguagePackType
-from lp.translations.model.distroseries_translations_copy import (
-    copy_active_translations,
-    )
 from lp.translations.model.distroserieslanguage import (
     DistroSeriesLanguage,
     DummyDistroSeriesLanguage,
@@ -1327,25 +1321,6 @@
                 BugSummary.sourcepackagename_id == None
                 )
 
-    def copyTranslationsFromParent(self, transaction, logger=None):
-        """See `IDistroSeries`."""
-        if logger is None:
-            logger = logging
-
-        assert self.defer_translation_imports, (
-            "defer_translation_imports not set!"
-            " That would corrupt translation data mixing new imports"
-            " with the information being copied.")
-
-        assert self.hide_all_translations, (
-            "hide_all_translations not set!"
-            " That would allow users to see and modify incomplete"
-            " translation state.")
-
-        flush_database_updates()
-        flush_database_caches()
-        copy_active_translations(self, transaction, logger)
-
     def getPOFileContributorsByLanguage(self, language):
         """See `IDistroSeries`."""
         contributors = IStore(Person).find(

=== modified file 'lib/lp/translations/doc/distroseries-translations-copy.txt'
--- lib/lp/translations/doc/distroseries-translations-copy.txt	2012-11-22 00:15:23 +0000
+++ lib/lp/translations/doc/distroseries-translations-copy.txt	2014-08-19 05:07:37 +0000
@@ -1,10 +1,11 @@
 Migrating translations between distro series
 ============================================
 
-When a new release series is created for a distribution it usually inherits
-all settings and data from its parent (preceding) series. To facilitate this
-for the translations the method copyTranslationsFromParent is called from a
-script once after the release series is created.
+When a new release series is created for a distribution it usually
+inherits all settings and data from its parent (preceding) series. To
+facilitate this for the translations the function
+copy_active_translations is called from a script once after the release
+series is created.
 
     >>> login("foo.bar@xxxxxxxxxxxxx")
     >>> foobuntu = factory.makeDistribution('foobuntu', 'Foobuntu')
@@ -68,40 +69,6 @@
     >>> transaction_stub = FakeTransaction()
 
 
-Preconditions for migrating translations between distro series
-==============================================================
-
-Before we are able to migrate translations, there are a set of preconditions
-that should be met:
-
-First one is that we should keep the new distroseries's translations hidden
-from the public, so the copying procedure is not confused by concurrent
-updates:
-
-    >>> carty.hide_all_translations = False
-    >>> carty.copyTranslationsFromParent(transaction_stub)
-    Traceback (most recent call last):
-    ...
-    AssertionError: hide_all_translations not set!...
-
-    # Set the field to TRUE so we meet this precondition for following
-    # tests.
-    >>> carty.hide_all_translations = True
-
-The other one is that, for the same reason, the import queue should not be
-accepting uploads for this distroseries.
-
-    >>> carty.defer_translation_imports = False
-    >>> carty.copyTranslationsFromParent(transaction_stub)
-    Traceback (most recent call last):
-    ...
-    AssertionError: defer_translation_imports not set!...
-
-    # Set the field to TRUE so we meet this precondition for following
-    # tests.
-    >>> carty.defer_translation_imports = True
-
-
 Performing the migration
 ========================
 
@@ -109,7 +76,10 @@
 and translation files from the parent. The actual translations, stored in
 POTMsgSet and TranslationMessage object, are shared between the two series.
 
-    >>> carty.copyTranslationsFromParent(transaction_stub)
+    >>> from lp.services.log.logger import DevNullLogger
+    >>> from lp.translations.model.distroseries_translations_copy import (
+    ...     copy_active_translations)
+    >>> copy_active_translations(carty, transaction_stub, DevNullLogger())
 
 All current templates were copied from the parent series but the deactivated
 template template3 was not copied.
@@ -155,11 +125,11 @@
     True
 
 
-Once the migration is done, copyTranslationsFromParent must not be called
+Once the migration is done, copy_active_translations must not be called
 again as it only operates on distroseries without any translation templates.
 Because of message sharing incremental copies are no longer needed.
 
-    >>> carty.copyTranslationsFromParent(transaction_stub)
+    >>> copy_active_translations(carty, transaction_stub, DevNullLogger())
     Traceback (most recent call last):
     ...
     AssertionError:
@@ -170,9 +140,9 @@
 ==================
 
 Now, we execute the script that will do the migration using
-copyTranslationsFromParent. For that we create a new child series to receive
-those translations. For testing purposes this series has translation imports
-enabled.
+copy_active_translations. For that we create a new child series to
+receive those translations. For testing purposes this series has
+translation imports enabled.
 
     >>> darty = factory.makeDistroSeries(
     ...     foobuntu, '99.2', name='darty', previous_series=barty)

=== modified file 'lib/lp/translations/scripts/copy_distroseries_translations.py'
--- lib/lp/translations/scripts/copy_distroseries_translations.py	2012-07-03 08:04:35 +0000
+++ lib/lp/translations/scripts/copy_distroseries_translations.py	2014-08-19 05:07:37 +0000
@@ -6,10 +6,18 @@
 __metaclass__ = type
 __all__ = ['copy_distroseries_translations']
 
+import logging
 
 from zope.component import getUtility
 
 from lp.registry.interfaces.distroseries import IDistroSeriesSet
+from lp.services.database.sqlbase import (
+    flush_database_caches,
+    flush_database_updates,
+    )
+from lp.translations.model.distroseries_translations_copy import (
+    copy_active_translations,
+    )
 
 
 class SeriesTranslationFlagsModified(Warning):
@@ -88,7 +96,15 @@
 
     try:
         # Do the actual work.
-        distroseries.copyTranslationsFromParent(txn, logger)
+        assert distroseries.defer_translation_imports, (
+            "defer_translation_imports not set!"
+            " That would corrupt translation data mixing new imports"
+            " with the information being copied.")
+        assert distroseries.hide_all_translations, (
+            "hide_all_translations not set!"
+            " That would allow users to see and modify incomplete"
+            " translation state.")
+        copy_active_translations(distroseries, txn, logger)
     except:
         copy_failed = True
         # Give us a fresh transaction for proper cleanup.