launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/copy-translations-duplicates into lp:launchpad.

Commit message:
Add a --skip-duplicates flag to copy-distroseries-translations to allow skipping sources that already exist in the target.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/copy-translations-duplicates/+merge/267171

Add a --skip-duplicates flag to copy-distroseries-translations to allow skipping sources that already exist in the target.

This lets us copy translations from ubuntu/wily to ubuntu-rtm/15.04, since the latter already contains some POTemplates and POFiles.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/copy-translations-duplicates into lp:launchpad.

=== modified file 'lib/lp/translations/model/distroseries_translations_copy.py'
--- lib/lp/translations/model/distroseries_translations_copy.py	2014-08-20 10:17:16 +0000
+++ lib/lp/translations/model/distroseries_translations_copy.py	2015-08-06 10:09:53 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Functions to copy translations from one distroseries to another."""
@@ -48,7 +48,7 @@
 
 
 def copy_active_translations(source, target, transaction, logger,
-                             sourcepackagenames=None):
+                             sourcepackagenames=None, skip_duplicates=False):
     """Populate target `DistroSeries` with source series' translations.
 
     The target must not already have any translations.
@@ -76,9 +76,14 @@
     full_name = "%s_%s" % (target.distribution.name, target.name)
     copier = MultiTableCopy(full_name, translation_tables, logger=logger)
 
-    # Incremental copy of updates is no longer supported
-    assert not target.has_translation_templates, (
-        "The target series must not yet have any translation templates.")
+    # Incremental copy of updates is no longer supported.  skip_duplicates
+    # is not a real incremental copy, since it doesn't update any
+    # POTemplates/POFiles for sources that already have a template in the
+    # target, but it is useful when digging ourselves out of situations
+    # where a few templates already exist in the target.
+    if not skip_duplicates:
+        assert not target.has_translation_templates, (
+            "The target series must not yet have any translation templates.")
 
     logger.info(
         "Populating blank distroseries %s %s with translations from %s %s." %
@@ -116,6 +121,12 @@
             where += (
                 ' AND sourcepackagename IN %s'
                 % quote([spn.id for spn in sourcepackagenames]))
+    if skip_duplicates:
+        where += ('''
+            AND sourcepackagename NOT IN (
+                SELECT sourcepackagename FROM potemplate
+                WHERE distroseries = %s)
+            ''' % quote(target))
     copier.extract('potemplate', [], where)
 
     # Now that we have the data "in private," where nobody else can see it,

=== modified file 'lib/lp/translations/scripts/copy_distroseries_translations.py'
--- lib/lp/translations/scripts/copy_distroseries_translations.py	2015-07-28 11:47:45 +0000
+++ lib/lp/translations/scripts/copy_distroseries_translations.py	2015-08-06 10:09:53 +0000
@@ -80,7 +80,8 @@
 def copy_distroseries_translations(source, target, txn, logger,
                                    published_sources_only=False,
                                    check_archive=None,
-                                   check_distroseries=None):
+                                   check_distroseries=None,
+                                   skip_duplicates=False):
     """Copy translations into a new `DistroSeries`.
 
     Wraps around `copy_active_translations`, but also ensures that the
@@ -128,7 +129,8 @@
         else:
             spns = None
         copy_active_translations(
-            source, target, txn, logger, sourcepackagenames=spns)
+            source, target, txn, logger, sourcepackagenames=spns,
+            skip_duplicates=skip_duplicates)
     except:
         copy_failed = True
         # Give us a fresh transaction for proper cleanup.

=== modified file 'lib/lp/translations/tests/test_distroseries_translations_copy.py'
--- lib/lp/translations/tests/test_distroseries_translations_copy.py	2014-08-20 05:24:19 +0000
+++ lib/lp/translations/tests/test_distroseries_translations_copy.py	2015-08-06 10:09:53 +0000
@@ -1,10 +1,13 @@
-# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2012-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Unit tests for distroseries translations initialization."""
 
 __metaclass__ = type
 
+from itertools import chain
+
+from testtools.matchers import ContainsAll
 import transaction
 from zope.component import getUtility
 
@@ -127,3 +130,49 @@
             dapper, feisty, transaction, DevNullLogger(),
             sourcepackagenames=spns)
         self.assertContentEqual(spns, get_template_spns(feisty))
+
+    def test_skip_duplicates(self):
+        # Normally the target distroseries must be empty.
+        # skip_duplicates=True works around this, simply by skipping any
+        # templates whose source package names match templates already in
+        # the target.
+        distro = self.factory.makeDistribution(name='notbuntu')
+        source_series = self.factory.makeDistroSeries(
+            distribution=distro, name='source')
+        target_series = self.factory.makeDistroSeries(
+            distribution=distro, name='target')
+        spns = [self.factory.makeSourcePackageName() for i in range(3)]
+        for spn in spns:
+            template = self.factory.makePOTemplate(
+                distroseries=source_series, sourcepackagename=spn)
+            self.factory.makePOFile(potemplate=template)
+        target_templates = []
+        target_pofiles = []
+        for spn in spns[:2]:
+            template = self.factory.makePOTemplate(
+                distroseries=target_series, sourcepackagename=spn)
+            target_templates.append(template)
+            target_pofiles.append(self.factory.makePOFile(potemplate=template))
+
+        def get_template_spns(series):
+            return [
+                pot.sourcepackagename for pot in
+                getUtility(IPOTemplateSet).getSubset(distroseries=series)]
+
+        self.assertContentEqual(spns[:2], get_template_spns(target_series))
+        self.assertRaises(
+            AssertionError, copy_active_translations,
+            source_series, target_series, transaction, DevNullLogger())
+        copy_active_translations(
+            source_series, target_series, transaction, DevNullLogger(),
+            skip_duplicates=True)
+        self.assertContentEqual(spns, get_template_spns(target_series))
+        # The original POTemplates in the target distroseries are untouched,
+        # along with their POFiles.
+        self.assertThat(
+            list(getUtility(IPOTemplateSet).getSubset(
+                distroseries=target_series)),
+            ContainsAll(target_templates))
+        self.assertContentEqual(
+            target_pofiles,
+            chain.from_iterable(pot.pofiles for pot in target_templates))

=== modified file 'scripts/copy-distroseries-translations.py'
--- scripts/copy-distroseries-translations.py	2015-07-28 11:47:45 +0000
+++ scripts/copy-distroseries-translations.py	2015-08-06 10:09:53 +0000
@@ -65,6 +65,12 @@
             action="store_true", default=False,
             help="Don't check if target's UI and imports are blocked; "
                  "actively block them.")
+        self.parser.add_option('--skip-duplicates', dest='skip_duplicates',
+            action="store_true", default=False,
+            help=(
+                "Allow the target distroseries to have some translation "
+                "templates; skip any templates and translations for "
+                "sources that already have a template in the target."))
 
     def main(self):
         target = getUtility(IDistributionSet)[self.options.distro][
@@ -111,7 +117,8 @@
         copy_distroseries_translations(
             source, target, self.txn, self.logger,
             published_sources_only=self.options.published_sources_only,
-            check_archive=check_archive, check_distroseries=check_distroseries)
+            check_archive=check_archive, check_distroseries=check_distroseries,
+            skip_duplicates=self.options.skip_duplicates)
 
         # We would like to update the DistroRelase statistics, but it takes
         # too long so this should be done after.