launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #01914
[Merge] lp:~danilo/launchpad/drop-variants into lp:launchpad
Данило Шеган has proposed merging lp:~danilo/launchpad/drop-variants into lp:launchpad.
Requested reviews:
Stuart Bishop (stub): db
Launchpad code reviewers (launchpad-reviewers)
= Removal of variants =
We've long ago done away with TranslationMessage.variant and POFile.variant usage, but because of the migration, we haven't really dropped the DB columns. Because of the migration script, we've also had to leave interface and model definitions in.
Now we can finally get rid of it all. When we drop DB columns, we also lose all the indexes that refer to it. They are critical to LP Translations performance so we have to recreate them.
However, since new index creation takes a long time (one test run on staging took ~20 minutes, though it used to take even longer in the past so it's hard to estimate), I split it all into two DB patches: one that creates the new indexes without referencing any of the variant fields, and another which drops the columns. New indexes do not conflict existing ones, so we can manually apply the first of the patches to the production DBs (both master and slaves, I am sure Stuart knows best how to do that), thus avoiding the need to run long DB update during rollout.
= Timings from staging =
CREATE UNIQUE INDEX pofile__potemplate__language__idx
ON pofile USING btree (potemplate, language);
Time: 8203.280 ms
CREATE UNIQUE INDEX tm__potmsgset__language__shared__current__key ON translationmessage USING btree (potmsgset, language) WHERE (((is_current IS TRUE) AND (potemplate IS NULL)));
Time: 256642.654 ms
CREATE UNIQUE INDEX tm__potmsgset__language__shared__imported__key ON translationmessage USING btree (potmsgset, language) WHERE (((is_imported IS TRUE) AND (potemplate IS NULL)));
Time: 190977.439 ms
CREATE INDEX tm__potmsgset__language__not_used__idx ON translationmessage USING btree (potmsgset, language) WHERE (NOT ((is_current IS TRUE) AND (is_imported IS TRUE)));
Time: 114520.636 ms
CREATE UNIQUE INDEX tm__potmsgset__potemplate__language__diverged__current__idx ON translationmessage USING btree (potmsgset, potemplate, language) WHERE (((is_current IS TRUE) AND (potemplate IS NOT NULL)));
Time: 103551.519 ms
CREATE UNIQUE INDEX tm__potmsgset__potemplate__language__diverged__imported__idx ON translationmessage USING btree (potmsgset, potemplate, language) WHERE (((is_imported IS TRUE) AND (potemplate IS NOT NULL)));
Time: 82104.305 ms
CREATE INDEX translationmessage__language__submitter__idx ON translationmessage USING btree (language, submitter);
Time: 463969.586 ms
--
https://code.launchpad.net/~danilo/launchpad/drop-variants/+merge/40735
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~danilo/launchpad/drop-variants into lp:launchpad.
=== added file 'database/schema/patch-2208-99-0.sql'
--- database/schema/patch-2208-99-0.sql 1970-01-01 00:00:00 +0000
+++ database/schema/patch-2208-99-0.sql 2010-11-12 15:56:52 +0000
@@ -0,0 +1,24 @@
+-- Copyright 2010 Canonical Ltd. This software is licensed under the
+-- GNU Affero General Public License version 3 (see the file LICENSE).
+SET client_min_messages=ERROR;
+
+-- Create new indexes to replace those using 'variant' column on either
+-- TranslationMessage or POFile tables. Done as a separate patch so it
+-- can be run on production DBs separately from actual DROP COLUMNs.
+
+CREATE UNIQUE INDEX pofile__potemplate__language__idx
+ ON pofile USING btree (potemplate, language);
+
+CREATE UNIQUE INDEX tm__potmsgset__language__shared__current__key ON translationmessage USING btree (potmsgset, language) WHERE (((is_current IS TRUE) AND (potemplate IS NULL)));
+
+CREATE UNIQUE INDEX tm__potmsgset__language__shared__imported__key ON translationmessage USING btree (potmsgset, language) WHERE (((is_imported IS TRUE) AND (potemplate IS NULL)));
+
+CREATE INDEX tm__potmsgset__language__not_used__idx ON translationmessage USING btree (potmsgset, language) WHERE (NOT ((is_current IS TRUE) AND (is_imported IS TRUE)));
+
+CREATE UNIQUE INDEX tm__potmsgset__potemplate__language__diverged__current__idx ON translationmessage USING btree (potmsgset, potemplate, language) WHERE (((is_current IS TRUE) AND (potemplate IS NOT NULL)));
+
+CREATE UNIQUE INDEX tm__potmsgset__potemplate__language__diverged__imported__idx ON translationmessage USING btree (potmsgset, potemplate, language) WHERE (((is_imported IS TRUE) AND (potemplate IS NOT NULL)));
+
+CREATE INDEX translationmessage__language__submitter__idx ON translationmessage USING btree (language, submitter);
+
+INSERT INTO LaunchpadDatabaseRevision VALUES (2208, 99, 0);
=== added file 'database/schema/patch-2208-99-1.sql'
--- database/schema/patch-2208-99-1.sql 1970-01-01 00:00:00 +0000
+++ database/schema/patch-2208-99-1.sql 2010-11-12 15:56:52 +0000
@@ -0,0 +1,10 @@
+-- Copyright 2010 Canonical Ltd. This software is licensed under the
+-- GNU Affero General Public License version 3 (see the file LICENSE).
+SET client_min_messages=ERROR;
+
+ALTER TABLE TranslationMessage
+ DROP COLUMN variant;
+ALTER TABLE POFile
+ DROP COLUMN variant;
+
+INSERT INTO LaunchpadDatabaseRevision VALUES (2208, 99, 1);
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2010-11-08 17:17:45 +0000
+++ lib/lp/testing/factory.py 2010-11-12 15:56:52 +0000
@@ -2361,7 +2361,7 @@
return template
def makePOFile(self, language_code=None, potemplate=None, owner=None,
- create_sharing=False, language=None, variant=None):
+ create_sharing=False, language=None):
"""Make a new translation file."""
assert language_code is None or language is None, (
"Please specifiy only one of language_code and language.")
@@ -2371,11 +2371,8 @@
language_code = language.code
if potemplate is None:
potemplate = self.makePOTemplate(owner=owner)
- pofile = potemplate.newPOFile(language_code,
- create_sharing=create_sharing)
- if variant is not None:
- removeSecurityProxy(pofile).variant = variant
- return pofile
+ return potemplate.newPOFile(language_code,
+ create_sharing=create_sharing)
def makePOTMsgSet(self, potemplate, singular=None, plural=None,
context=None, sequence=0):
=== modified file 'lib/lp/translations/interfaces/pofile.py'
--- lib/lp/translations/interfaces/pofile.py 2010-11-05 14:56:34 +0000
+++ lib/lp/translations/interfaces/pofile.py 2010-11-12 15:56:52 +0000
@@ -66,9 +66,6 @@
title=_('Language of this PO file.'),
vocabulary='Language', required=True)
- variant = TextLine(
- title=_('The language variant for this translation file.'))
-
title = TextLine(
title=_('The translation file title.'), required=True, readonly=True)
=== modified file 'lib/lp/translations/interfaces/translationmessage.py'
--- lib/lp/translations/interfaces/translationmessage.py 2010-10-04 22:56:09 +0000
+++ lib/lp/translations/interfaces/translationmessage.py 2010-11-12 15:56:52 +0000
@@ -115,9 +115,6 @@
title=_('Language of this translation message.'),
vocabulary='Language', required=False)
- variant = TextLine(
- title=_('The language variant for this translation file.'))
-
potmsgset = Object(
title=_("The template message that this translation is for"),
readonly=True, required=True, schema=IPOTMsgSet)
=== modified file 'lib/lp/translations/model/pofile.py'
--- lib/lp/translations/model/pofile.py 2010-11-09 08:43:34 +0000
+++ lib/lp/translations/model/pofile.py 2010-11-12 15:56:52 +0000
@@ -479,9 +479,6 @@
language = ForeignKey(foreignKey='Language',
dbName='language',
notNull=True)
- variant = StringCol(dbName='variant',
- notNull=False,
- default=None)
description = StringCol(dbName='description',
notNull=False,
default=None)
=== modified file 'lib/lp/translations/model/translationmessage.py'
--- lib/lp/translations/model/translationmessage.py 2010-11-11 11:14:34 +0000
+++ lib/lp/translations/model/translationmessage.py 2010-11-12 15:56:52 +0000
@@ -120,7 +120,6 @@
self.browser_pofile = pofile
self.potemplate = pofile.potemplate
self.language = pofile.language
- self.variant = None
self.potmsgset = potmsgset
UTC = pytz.timezone('UTC')
self.date_created = datetime.now(UTC)
@@ -235,9 +234,6 @@
default=None)
language = ForeignKey(
foreignKey='Language', dbName='language', notNull=False, default=None)
- variant = StringCol(dbName='variant',
- notNull=False,
- default=None)
potmsgset = ForeignKey(
foreignKey='POTMsgSet', dbName='potmsgset', notNull=True)
date_created = UtcDateTimeCol(
=== removed file 'lib/lp/translations/scripts/migrate_variants.py'
--- lib/lp/translations/scripts/migrate_variants.py 2010-11-05 09:16:14 +0000
+++ lib/lp/translations/scripts/migrate_variants.py 1970-01-01 00:00:00 +0000
@@ -1,198 +0,0 @@
-# Copyright 2010 Canonical Ltd. This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""Remove 'variant' usage in POFiles and TranslationMessages."""
-
-__metaclass__ = type
-__all__ = ['MigrateVariantsProcess']
-
-import logging
-
-from zope.component import getUtility
-from zope.interface import implements
-
-from canonical.launchpad.interfaces.looptuner import ITunableLoop
-from canonical.launchpad.utilities.looptuner import DBLoopTuner
-from canonical.launchpad.webapp.interfaces import (
- IStoreSelector,
- MAIN_STORE,
- MASTER_FLAVOR,
- )
-from lp.services.worlddata.interfaces.language import ILanguageSet
-from lp.services.worlddata.model.language import Language
-from lp.translations.model.translationmessage import TranslationMessage
-
-
-class ReplacerMixin:
- """Replaces `language` and `variant` on all contained objects."""
-
- def __init__(self, transaction, logger, title, contents, new_language):
- self.transaction = transaction
- self.logger = logger
- self.start_at = 0
-
- self.title = title
- self.language = new_language
- self.contents = list(contents)
- self.logger.info(
- "Figuring out %ss that need fixing: "
- "this may take a while..." % title)
- self.total = len(self.contents)
- self.logger.info(
- "Fixing up a total of %d %ss." % (self.total, self.title))
- self.store = getUtility(IStoreSelector).get(MAIN_STORE, MASTER_FLAVOR)
-
- def isDone(self):
- """See `ITunableLoop`."""
- # When the main loop hits the end of the list of objects,
- # it sets start_at to None.
- return self.start_at is None
-
- def getNextBatch(self, chunk_size):
- """Return a batch of objects to work with."""
- end_at = self.start_at + int(chunk_size)
- self.logger.debug(
- "Getting %s[%d:%d]..." % (self.title, self.start_at, end_at))
- return self.contents[self.start_at: end_at]
-
- def __call__(self, chunk_size):
- """See `ITunableLoop`.
-
- Retrieve a batch of objects in ascending id order, and switch
- all of them to self.language and no variant.
- """
- object_ids = self.getNextBatch(chunk_size)
- # Avoid circular imports.
- from lp.translations.model.pofile import POFile
-
- if len(object_ids) == 0:
- self.start_at = None
- else:
- if self.title == 'TranslationMessage':
- results = self.store.find(
- TranslationMessage,
- TranslationMessage.id.is_in(object_ids))
- results.set(
- TranslationMessage.language==self.language, variant=None)
- else:
- results = self.store.find(
- POFile, POFile.id.is_in(object_ids))
- results.set(
- POFile.language==self.language, variant=None)
-
- self.transaction.commit()
- self.transaction.begin()
-
- self.start_at += len(object_ids)
- self.logger.info("Processed %d/%d of %s." % (
- self.start_at, self.total, self.title))
-
-
-class TranslationMessageVariantReplacer(ReplacerMixin):
- """Replaces language on all `TranslationMessage`s with variants."""
- implements(ITunableLoop)
-
- def __init__(self, transaction, logger, tm_ids, new_language):
- super(TranslationMessageVariantReplacer, self).__init__(
- transaction, logger, 'TranslationMessage',
- tm_ids, new_language)
-
-
-class POFileVariantReplacer(ReplacerMixin):
- """Replaces language on all `TranslationMessage`s with variants."""
- implements(ITunableLoop)
-
- def __init__(self, transaction, logger, pofile_ids, new_language):
- super(POFileVariantReplacer, self).__init__(
- transaction, logger, 'POFile', pofile_ids, new_language)
-
-
-class MigrateVariantsProcess:
- """Mark all `POFile` translation credits as translated."""
-
- def __init__(self, transaction, logger=None):
- self.transaction = transaction
- self.logger = logger
- if logger is None:
- self.logger = logging.getLogger("migrate-variants")
- self.store = getUtility(IStoreSelector).get(MAIN_STORE, MASTER_FLAVOR)
-
- def getPOFileIDsForLanguage(self, language, variant):
- # Avoid circular imports.
- from lp.translations.model.pofile import POFile
- return self.store.find(POFile.id,
- POFile.languageID == language.id,
- POFile.variant == variant)
-
- def getTranslationMessageIDsForLanguage(self, language, variant):
- return self.store.find(TranslationMessage.id,
- TranslationMessage.languageID == language.id,
- TranslationMessage.variant == variant)
-
- def getOrCreateLanguage(self, language, variant):
- """Create a language based on `language` and variant.
-
- Resulting language keeps the properties of parent language,
- but has a language code appended with the `variant`.
- """
- language_set = getUtility(ILanguageSet)
- new_code = '%s@%s' % (language.code, variant)
- new_language = language_set.getLanguageByCode(new_code)
- if new_language is None:
- new_language = language_set.createLanguage(
- new_code,
- englishname='%s %s' % (language.englishname, variant),
- pluralforms=language.pluralforms,
- pluralexpression=language.pluralexpression,
- visible=False,
- direction=language.direction)
- self.logger.info("Created language %s." % new_code)
- return new_language
-
- def fetchAllLanguagesWithVariants(self):
- from lp.translations.model.pofile import POFile
- pofile_language_variants = self.store.find(
- (Language, POFile.variant),
- POFile.languageID==Language.id,
- POFile.variant!=None)
- translationmessage_language_variants = self.store.find(
- (Language, TranslationMessage.variant),
- TranslationMessage.languageID==Language.id,
- TranslationMessage.variant!=None)
-
- # XXX DaniloSegan 2010-07-26: ideally, we'd use an Union of
- # these two ResultSets, however, Storm doesn't treat two
- # columns of the same type on different tables as 'compatible'
- # (bug #610492).
- language_variants = set([])
- for language_variant in pofile_language_variants:
- language_variants.add(language_variant)
- for language_variant in translationmessage_language_variants:
- language_variants.add(language_variant)
- return language_variants
-
- def run(self):
- language_variants = self.fetchAllLanguagesWithVariants()
- if len(language_variants) == 0:
- self.logger.info("Nothing to do.")
- for language, variant in language_variants:
- self.logger.info(
- "Migrating %s (%s@%s)..." % (
- language.englishname, language.code, variant))
- new_language = self.getOrCreateLanguage(language, variant)
-
- tm_ids = self.getTranslationMessageIDsForLanguage(
- language, variant)
- tm_loop = TranslationMessageVariantReplacer(
- self.transaction, self.logger,
- tm_ids, new_language)
- DBLoopTuner(tm_loop, 5, minimum_chunk_size=100).run()
-
- pofile_ids = self.getPOFileIDsForLanguage(
- language, variant)
- pofile_loop = POFileVariantReplacer(
- self.transaction, self.logger,
- pofile_ids, new_language)
- DBLoopTuner(pofile_loop, 5, minimum_chunk_size=10).run()
-
- self.logger.info("Done.")
=== removed file 'lib/lp/translations/scripts/tests/test_migrate_variants.py'
--- lib/lp/translations/scripts/tests/test_migrate_variants.py 2010-10-04 19:50:45 +0000
+++ lib/lp/translations/scripts/tests/test_migrate_variants.py 1970-01-01 00:00:00 +0000
@@ -1,139 +0,0 @@
-# Copyright 2010 Canonical Ltd. This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-__metaclass__ = type
-
-from zope.component import getUtility
-
-from canonical.testing.layers import LaunchpadZopelessLayer
-from lp.services.worlddata.interfaces.language import ILanguageSet
-from lp.testing import TestCaseWithFactory
-from lp.translations.scripts.migrate_variants import MigrateVariantsProcess
-
-
-class TestMigrateVariants(TestCaseWithFactory):
- """Test variant migration."""
- layer = LaunchpadZopelessLayer
-
- def setUp(self):
- # This test needs the privileges of rosettaadmin (to update
- # TranslationMessages) but it also needs to set up test conditions
- # which requires other privileges.
- self.layer.switchDbUser('postgres')
- super(TestMigrateVariants, self).setUp(user='mark@xxxxxxxxxxx')
- self.migrate_process = MigrateVariantsProcess(self.layer.txn)
- self.language_set = getUtility(ILanguageSet)
-
- def test_fetchAllLanguagesWithVariants(self):
- all_langs = self.migrate_process.fetchAllLanguagesWithVariants()
- self.assertContentEqual([], all_langs)
-
- def _sortLanguageVariantPairs(self, lang_vars):
- cmp_pairs = lambda a, b: cmp(a[0].code, b[0].code) or cmp(a[1], b[1])
- return sorted(lang_vars, cmp=cmp_pairs)
-
- def assertLanguageListsEqual(self, a, b):
- # We provide our own assertion because assertContentEqual does
- # only naive sorting and that doesn't always work with tuples
- # which might have an identical first element, but differ on
- # the second element.
- self.assertEqual(
- self._sortLanguageVariantPairs(a),
- self._sortLanguageVariantPairs(b))
-
- def test_fetchAllLanguagesWithVariants_pofiles(self):
- serbian_pofile = self.factory.makePOFile('sr', variant=u'test')
- serbian = serbian_pofile.language
- self.layer.txn.commit()
- all_langs = self.migrate_process.fetchAllLanguagesWithVariants()
- self.assertLanguageListsEqual(
- [(serbian, u'test')], all_langs)
-
- def test_fetchAllLanguagesWithVariants_pofile_and_translation(self):
- # With both a POFile and TranslationMessage for the same language
- # and variant, the pair is returned only once.
- serbian_pofile = self.factory.makePOFile('sr', variant=u'test')
- serbian = serbian_pofile.language
- message = self.factory.makeTranslationMessage(pofile=serbian_pofile)
- self.layer.txn.commit()
-
- all_langs = self.migrate_process.fetchAllLanguagesWithVariants()
- self.assertLanguageListsEqual(
- [(serbian, u'test')], all_langs)
-
- def test_fetchAllLanguagesWithVariants_translationmessages(self):
- # We create a TranslationMessage with no matching PO file
- # by first creating it attached to sr@test POFile, and then
- # directly changing the variant on it.
- serbian_pofile = self.factory.makePOFile('sr', variant=u'test')
- serbian = serbian_pofile.language
- message = self.factory.makeTranslationMessage(pofile=serbian_pofile)
- message.variant = u'another'
- self.layer.txn.commit()
-
- all_langs = self.migrate_process.fetchAllLanguagesWithVariants()
- self.assertLanguageListsEqual(
- [(serbian, u'test'), (serbian, u'another')],
- all_langs)
-
- def test_getOrCreateLanguage_new(self):
- serbian = self.language_set.getLanguageByCode('sr')
- new_language = self.migrate_process.getOrCreateLanguage(
- serbian, u'test')
- self.assertEqual(u'sr@test', new_language.code)
- self.assertEqual(u'Serbian test', new_language.englishname)
- self.assertEqual(serbian.pluralforms, new_language.pluralforms)
- self.assertEqual(serbian.pluralexpression,
- new_language.pluralexpression)
- self.assertEqual(serbian.direction, new_language.direction)
-
- def test_getOrCreateLanguage_noop(self):
- serbian = self.language_set.getLanguageByCode('sr')
- serbian_test = self.language_set.createLanguage(
- 'sr@test', 'Serbian test')
- new_language = self.migrate_process.getOrCreateLanguage(
- serbian, u'test')
- self.assertEqual(serbian_test, new_language)
-
- def test_getPOFileIDsForLanguage_none(self):
- serbian = self.language_set.getLanguageByCode('sr')
- pofile_ids = self.migrate_process.getPOFileIDsForLanguage(
- serbian, u'test')
- self.assertContentEqual([], list(pofile_ids))
-
- def test_getPOFileIDsForLanguage_variant(self):
- serbian = self.language_set.getLanguageByCode('sr')
- sr_pofile = self.factory.makePOFile(serbian.code, variant=u'test')
- pofile_ids = self.migrate_process.getPOFileIDsForLanguage(
- serbian, u'test')
- self.assertContentEqual([sr_pofile.id], list(pofile_ids))
-
- def test_getPOFileIDsForLanguage_nonvariant(self):
- serbian = self.language_set.getLanguageByCode('sr')
- sr_pofile = self.factory.makePOFile(serbian.code, variant=None)
- pofile_ids = self.migrate_process.getPOFileIDsForLanguage(
- serbian, u'test')
- self.assertContentEqual([], list(pofile_ids))
-
- def test_getTranslationMessageIDsForLanguage_none(self):
- serbian = self.language_set.getLanguageByCode('sr')
- tm_ids = self.migrate_process.getTranslationMessageIDsForLanguage(
- serbian, u'test')
- self.assertContentEqual([], list(tm_ids))
-
- def test_getTranslationMessageIDsForLanguage_variant(self):
- serbian = self.language_set.getLanguageByCode('sr')
- sr_pofile = self.factory.makePOFile(serbian.code, variant=u'test')
- message = self.factory.makeTranslationMessage(sr_pofile)
- message.variant = u'test'
- tm_ids = self.migrate_process.getTranslationMessageIDsForLanguage(
- serbian, u'test')
- self.assertContentEqual([message.id], list(tm_ids))
-
- def test_getTranslationMessageIDsForLanguage_nonvariant(self):
- serbian = self.language_set.getLanguageByCode('sr')
- sr_pofile = self.factory.makePOFile(serbian.code, variant=None)
- message = self.factory.makeTranslationMessage(sr_pofile)
- tm_ids = self.migrate_process.getTranslationMessageIDsForLanguage(
- serbian, u'test')
- self.assertContentEqual([], list(tm_ids))
=== removed file 'scripts/rosetta/migrate_variants.py'
--- scripts/rosetta/migrate_variants.py 2010-07-26 03:19:39 +0000
+++ scripts/rosetta/migrate_variants.py 1970-01-01 00:00:00 +0000
@@ -1,30 +0,0 @@
-#!/usr/bin/python -S
-#
-# Copyright 2010 Canonical Ltd. This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""Migrate all objects specifying variants to appropriate languages."""
-
-import _pythonpath
-
-from lp.services.scripts.base import LaunchpadScript
-from lp.translations.scripts.migrate_variants import (
- MigrateVariantsProcess)
-
-
-class MigrateVariants(LaunchpadScript):
- """Go through all POFiles and TranslationMessages and get rid of variants.
-
- Replaces use of `variant` field with a new language with the code
- corresponding to the 'previous language'@'variant'.
- """
-
- def main(self):
- fixer = MigrateVariantsProcess(self.txn, self.logger)
- fixer.run()
-
-
-if __name__ == '__main__':
- script = MigrateVariants(name="migratevariants",
- dbuser='rosettaadmin')
- script.lock_and_run()
Follow ups
