launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02555
[Merge] lp:~abentley/launchpad/refactor-migration into lp:launchpad
Aaron Bentley has proposed merging lp:~abentley/launchpad/refactor-migration into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~abentley/launchpad/refactor-migration/+merge/49086
= Summary =
Make message-sharing-migration script reusable for when new Packagings are
created.
== Proposed fix ==
Extract a TranslationMerger from the message-sharing-migration script that can
be reused in a TranslationMerge Job.
== Pre-implementation notes ==
None
== Implementation details ==
I made some style changes along the way, e.g. marking methods as classmethods
or staticmethods if they didn't use instance variables. I also switched from
"record_statements" to StormStatementRecorder, which I find more readable.
Even though I renamed scripts/message_sharing_migration.py to
translationmerger.py, I kept the MessageSharingMerge class in the same file, to
make the actual changes more obvious, and to retain annotation history.
I also extracted a TransactionManager class, because otherwise, both the
MessageSharingMerge and the TranslationMerger would have needed to provide this
kind of functionality.
== Tests ==
bin/test -vt test_translationmerger -t message-sharing-merge-script.txt
== Demo and Q/A ==
None; refactoring
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/translations/tests/test_translationmerger.py
lib/lp/translations/translationmerger.py
scripts/rosetta/message-sharing-merge.py
./scripts/rosetta/message-sharing-merge.py
10: '_pythonpath' imported but unused
24: E303 too many blank lines (3)
--
https://code.launchpad.net/~abentley/launchpad/refactor-migration/+merge/49086
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/refactor-migration into lp:launchpad.
=== renamed file 'lib/lp/translations/scripts/tests/test_message_sharing_migration.py' => 'lib/lp/translations/tests/test_translationmerger.py'
--- lib/lp/translations/scripts/tests/test_message_sharing_migration.py 2011-01-26 19:28:48 +0000
+++ lib/lp/translations/tests/test_translationmerger.py 2011-02-09 16:43:57 +0000
@@ -15,7 +15,7 @@
from canonical.testing.layers import LaunchpadZopelessLayer
from lp.services.worlddata.interfaces.language import ILanguageSet
from lp.testing import (
- record_statements,
+ StormStatementRecorder,
TestCaseWithFactory,
)
from lp.testing.sampledata import ADMIN_EMAIL
@@ -23,8 +23,10 @@
from lp.translations.model.pomsgid import POMsgID
from lp.translations.model.potemplate import POTemplate
from lp.translations.model.potranslation import POTranslation
-from lp.translations.scripts.message_sharing_migration import (
+from lp.translations.translationmerger import (
MessageSharingMerge,
+ TransactionManager,
+ TranslationMerger,
)
@@ -53,6 +55,8 @@
self.script = MessageSharingMerge('tms-merging-test', test_args=[])
self.script.logger.setLevel(ERROR)
+ tm = TransactionManager(self.script.txn, self.script.options.dry_run)
+ self.merger = TranslationMerger(self.templates, tm)
class TestPOTMsgSetMerging(TestCaseWithFactory, TranslatableProductMixin):
@@ -76,7 +80,7 @@
stable_potmsgset = self.factory.makePOTMsgSet(
self.stable_template, singular='foo', sequence=1)
- self.script._mergePOTMsgSets(self.templates)
+ self.merger.mergePOTMsgSets()
trunk_messages = list(self.trunk_template.getPOTMsgSets(False))
stable_messages = list(self.stable_template.getPOTMsgSets(False))
@@ -92,8 +96,8 @@
stable_potmsgset = self.factory.makePOTMsgSet(
self.stable_template, singular='foo', sequence=1)
- self.script._mergePOTMsgSets(self.templates)
- self.script._mergePOTMsgSets(self.templates)
+ self.merger.mergePOTMsgSets()
+ self.merger.mergePOTMsgSets()
trunk_messages = list(self.trunk_template.getPOTMsgSets(False))
stable_messages = list(self.stable_template.getPOTMsgSets(False))
@@ -108,7 +112,7 @@
stable_potmsgset = self.factory.makePOTMsgSet(
self.stable_template, singular='foo', context='bar', sequence=1)
- self.script._mergePOTMsgSets(self.templates)
+ self.merger.mergePOTMsgSets()
trunk_messages = list(self.trunk_template.getPOTMsgSets(False))
stable_messages = list(self.stable_template.getPOTMsgSets(False))
@@ -125,7 +129,7 @@
self.factory.makePOTMsgSet(
self.stable_template, singular='foo', sequence=9)
- self.script._mergePOTMsgSets(self.templates)
+ self.merger.mergePOTMsgSets()
trunk_potmsgset = self.trunk_template.getPOTMsgSetByMsgIDText('foo')
stable_potmsgset = self.stable_template.getPOTMsgSetByMsgIDText('foo')
@@ -251,7 +255,7 @@
* Two templates for the "trunk" and "stable" release series.
* Matching POTMsgSets for the string "foo" in each.
- The matching POTMsgSets will be merged by the _mergePOTMsgSets
+ The matching POTMsgSets will be merged by the mergePOTMsgSets
call.
"""
super(TestPOTMsgSetMergingAndTranslations, self).setUp(
@@ -267,7 +271,7 @@
trunk_message.is_current_upstream = True
stable_message.is_current_upstream = True
- self.script._mergePOTMsgSets(self.templates)
+ self.merger.mergePOTMsgSets()
self.assertEqual(self._getTranslations(), ('bar', 'splat'))
self.assertEqual(self._getMessages(), (trunk_message, stable_message))
@@ -281,7 +285,7 @@
trunk_message.is_current_upstream = True
stable_message.is_current_upstream = True
- self.script._mergePOTMsgSets(self.templates)
+ self.merger.mergePOTMsgSets()
self.assertEqual(self._getTranslations(), ('bar', 'bar'))
@@ -294,7 +298,7 @@
trunk_message.is_current_upstream = True
stable_message.is_current_upstream = True
- self.script._mergePOTMsgSets(self.templates)
+ self.merger.mergePOTMsgSets()
# The POTMsgSets are now merged.
potmsgset = self.trunk_template.getPOTMsgSetByMsgIDText('foo')
@@ -317,7 +321,7 @@
trunk_message.is_current_upstream = False
stable_message.is_current_upstream = False
- self.script._mergePOTMsgSets(self.templates)
+ self.merger.mergePOTMsgSets()
# Having these suggestions does not mean that there are current
# translations.
@@ -331,7 +335,7 @@
trunk_message.is_current_upstream = True
stable_message.is_current_upstream = True
- self.script._mergePOTMsgSets(self.templates)
+ self.merger.mergePOTMsgSets()
trunk_message, stable_message = self._getMessages()
self.assertEqual(trunk_message.potemplate, None)
@@ -356,7 +360,7 @@
self.assertEqual(self._getTranslations(), ('bzo', 'smurf'))
- self.script._mergePOTMsgSets(self.templates)
+ self.merger.mergePOTMsgSets()
# The current translations stay as they are.
self.assertEqual(self._getTranslations(), ('bzo', 'smurf'))
@@ -386,7 +390,7 @@
# be.
self._makeTranslationMessages('x', 'x')
- self.script._mergeTranslationMessages(self.templates)
+ self.merger.mergeTranslationMessages()
trunk_message, stable_message = self._getMessages()
self.assertNotEqual(trunk_message, stable_message)
@@ -414,8 +418,8 @@
self._makeTranslationMessages(
'a', 'b', trunk_diverged=True, stable_diverged=True)
- self.script._mergePOTMsgSets(self.templates)
- self.script._mergeTranslationMessages(self.templates)
+ self.merger.mergePOTMsgSets()
+ self.merger.mergeTranslationMessages()
# Translations for the existing templates stay as they are.
self.assertEqual(self._getTranslations(), ('a', 'b'))
@@ -430,8 +434,8 @@
self._makeTranslationMessages(
'x', 'x', trunk_diverged=True, stable_diverged=True)
- self.script._mergePOTMsgSets(self.templates)
- self.script._mergeTranslationMessages(self.templates)
+ self.merger.mergePOTMsgSets()
+ self.merger.mergeTranslationMessages()
trunk_message, stable_message = self._getMessages()
self.assertEqual(trunk_message, stable_message)
@@ -453,8 +457,8 @@
trunk_message.is_current_upstream = False
stable_message.is_current_upstream = False
- self.script._mergePOTMsgSets(self.templates)
- self.script._mergeTranslationMessages(self.templates)
+ self.merger.mergePOTMsgSets()
+ self.merger.mergeTranslationMessages()
# Translations for the existing templates stay as they are.
self.assertEqual(self._getTranslations(), (None, None))
@@ -470,8 +474,8 @@
self._makeTranslationMessages(
'ips', 'unq', trunk_diverged=True, stable_diverged=False)
- self.script._mergePOTMsgSets(self.templates)
- self.script._mergeTranslationMessages(self.templates)
+ self.merger.mergePOTMsgSets()
+ self.merger.mergeTranslationMessages()
# Translations for the existing templates stay as they are.
self.assertEqual(self._getTranslations(), ('ips', 'unq'))
@@ -491,8 +495,8 @@
self.assertEqual(self._getTranslations(), ('n', None))
- self.script._mergePOTMsgSets(self.templates)
- self.script._mergeTranslationMessages(self.templates)
+ self.merger.mergePOTMsgSets()
+ self.merger.mergeTranslationMessages()
# The less-representative POTMsgSet gains a translation, because
# it now uses the shared translation.
@@ -553,8 +557,8 @@
# Now the migration script runs. This also carries the
# POFileTranslator record for stable_message into trunk_pofile.
# The one for contented_message disappears in the process.
- self.script._mergePOTMsgSets(self.templates)
- self.script._mergeTranslationMessages(self.templates)
+ self.merger.mergePOTMsgSets()
+ self.merger.mergeTranslationMessages()
poft = poftset.getForPersonPOFile(translator, self.trunk_pofile)
self.assertEqual(poft.latest_message, stable_message)
@@ -591,7 +595,7 @@
tms = set(potmsgset.getAllTranslationMessages())
self.assertEqual(tms, set([trunk_message, stable_message]))
- self.script._removeDuplicateMessages(self.templates)
+ self.merger._removeDuplicateMessages()
# The duplicates have been cleaned up.
self.assertEqual(potmsgset.getAllTranslationMessages().count(), 1)
@@ -611,7 +615,7 @@
pofile=self.trunk_pofile, potmsgset=self.trunk_potmsgset,
text='gbzidh', diverged=False)
- self.script._scrubPOTMsgSetTranslations(self.trunk_potmsgset)
+ self.merger._scrubPOTMsgSetTranslations(self.trunk_potmsgset)
message1, message2 = self._getMessages()
self.assertIsNot(None, message1)
@@ -634,7 +638,7 @@
message2.potemplate = self.trunk_template
ids = (message1.id, message2.id)
- self.script._scrubPOTMsgSetTranslations(self.trunk_potmsgset)
+ self.merger._scrubPOTMsgSetTranslations(self.trunk_potmsgset)
message, no_message = self._getMessages()
@@ -654,7 +658,7 @@
# potmsgset.
trunk_message, stable_message = self._makeTranslationMessages(
'ex', 'why', trunk_diverged=False, stable_diverged=False)
- ubuntu_clash, upstream_clash, twin = self.script._findClashes(
+ ubuntu_clash, upstream_clash, twin = self.merger._findClashes(
stable_message, self.trunk_potmsgset, None)
# Moving stable_message fully into trunk would clash with
@@ -677,7 +681,7 @@
message.is_current_upstream = False
message.is_current_ubuntu = True
- ubuntu_clash, upstream_clash, twin = self.script._findClashes(
+ ubuntu_clash, upstream_clash, twin = self.merger._findClashes(
stable_message, self.trunk_potmsgset, None)
self.assertEqual(upstream_clash, None)
@@ -691,7 +695,7 @@
'klob', 'klob', trunk_diverged=False, stable_diverged=False)
trunk_message.is_current_upstream = False
- ubuntu_clash, upstream_clash, twin = self.script._findClashes(
+ ubuntu_clash, upstream_clash, twin = self.merger._findClashes(
stable_message, self.trunk_potmsgset, None)
self.assertEqual(upstream_clash, None)
@@ -704,7 +708,7 @@
trunk_message, stable_message = self._makeTranslationMessages(
'sniw', 'sniw', trunk_diverged=False, stable_diverged=False)
- ubuntu_clash, upstream_clash, twin = self.script._findClashes(
+ ubuntu_clash, upstream_clash, twin = self.merger._findClashes(
stable_message, self.trunk_potmsgset, None)
self.assertEqual(upstream_clash, None)
@@ -720,7 +724,7 @@
current_message = self._makeTranslationMessage(
self.trunk_pofile, self.trunk_potmsgset, 'gah', False)
- ubuntu_clash, upstream_clash, twin = self.script._findClashes(
+ ubuntu_clash, upstream_clash, twin = self.merger._findClashes(
stable_message, self.trunk_potmsgset, None)
self.assertEqual(upstream_clash, current_message)
@@ -788,17 +792,19 @@
self._resetReferences()
self.assertNotEqual([], self.templates)
- _ignored, statements = record_statements(
- self.script._mergePOTMsgSets, self.templates)
- self.assertNoStatementsInvolvingTable(POMsgID._table, statements)
- self.assertNoStatementsInvolvingTable(
- POTranslation._table, statements)
+ with StormStatementRecorder() as recorder:
+ self.merger.mergePOTMsgSets()
+ self.assertNoStatementsInvolvingTable(
+ POMsgID._table, recorder.statements)
+ self.assertNoStatementsInvolvingTable(
+ POTranslation._table, recorder.statements)
- _ignored, statements = record_statements(
- self.script._mergeTranslationMessages, self.templates)
- self.assertNoStatementsInvolvingTable(POMsgID._table, statements)
- self.assertNoStatementsInvolvingTable(
- POTranslation._table, statements)
+ with StormStatementRecorder() as recorder:
+ self.merger.mergeTranslationMessages()
+ self.assertNoStatementsInvolvingTable(
+ POMsgID._table, recorder.statements)
+ self.assertNoStatementsInvolvingTable(
+ POTranslation._table, recorder.statements)
def test_suite():
=== renamed file 'lib/lp/translations/scripts/message_sharing_migration.py' => 'lib/lp/translations/translationmerger.py'
--- lib/lp/translations/scripts/message_sharing_migration.py 2011-01-26 19:28:48 +0000
+++ lib/lp/translations/translationmerger.py 2011-02-09 16:43:57 +0000
@@ -1,9 +1,11 @@
-# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
__all__ = [
'MessageSharingMerge',
+ 'TransactionManager',
+ 'TranslationMerger',
]
@@ -266,32 +268,54 @@
class_count = len(equivalence_classes)
log.info("Merging %d template equivalence classes." % class_count)
+ tm = TransactionManager(self.txn, self.options.dry_run)
for number, name in enumerate(sorted(equivalence_classes.iterkeys())):
templates = equivalence_classes[name]
log.info(
"Merging equivalence class '%s': %d template(s) (%d / %d)" % (
name, len(templates), number + 1, class_count))
log.debug("Templates: %s" % str(templates))
-
+ merger = TranslationMerger(templates, tm)
if self.options.remove_duplicates:
log.info("Removing duplicate messages.")
- self._removeDuplicateMessages(templates)
- self._endTransaction(intermediate=True)
+ merger.removeDuplicateMessages()
+ tm.endTransaction(intermediate=True)
if self.options.merge_potmsgsets:
log.info("Merging POTMsgSets.")
- self._mergePOTMsgSets(templates)
- self._endTransaction(intermediate=True)
+ merger.mergePOTMsgSets()
+ tm.endTransaction(intermediate=True)
if self.options.merge_translationmessages:
log.info("Merging TranslationMessages.")
- self._mergeTranslationMessages(templates)
+ merger.mergeTranslationMessages()
- self._endTransaction()
+ tm.endTransaction()
log.info("Done.")
- def _endTransaction(self, intermediate=False):
+
+class TransactionManager:
+ """Manage transactions for script runs.
+
+ For normal operation, every tenth intermediate transaction is actually
+ committed. At the end, the final transaction is committed.
+
+ For dry runs, no transactions are committed and the final transaction is
+ aborted.
+ """
+
+ def __init__(self, txn, dry_run):
+ """Constructor.
+
+ :param txn: The transaction to commit or abort.
+ :param dry_run: If true, this is a dry run.
+ """
+ self.txn = txn
+ self.dry_run = dry_run
+ self.commit_count = 0
+
+ def endTransaction(self, intermediate=False):
"""End this transaction and start a new one.
:param intermediate: Whether this is an intermediate commit.
@@ -308,38 +332,49 @@
if intermediate and self.commit_count % 10 != 0:
return
- if self.options.dry_run:
+ if self.dry_run:
if not intermediate:
self.txn.abort()
else:
self.txn.commit()
- def _removeDuplicateMessages(self, potemplates):
+
+class TranslationMerger:
+ """Merge translations across a set of potemplates."""
+
+ def __init__(self, potemplates, tm):
+ """Constructor.
+
+ :param potemplates: The templates to merge across.
+ """
+ self.template_set = getUtility(IPOTemplateSet)
+ self.compare_template_precedence = (
+ self.template_set.compareSharingPrecedence)
+ self.potemplates = potemplates
+ self.tm = tm
+
+ def _removeDuplicateMessages(self):
"""Get rid of duplicate `TranslationMessages` where needed."""
- self._setUpUtilities()
-
representatives = {}
order_check = OrderingCheck(cmp=self.compare_template_precedence)
- for template in potemplates:
+ for template in self.potemplates:
order_check.check(template)
for potmsgset in template.getPOTMsgSets(False, prefetch=False):
key = get_potmsgset_key(potmsgset)
if key not in representatives:
representatives[key] = potmsgset.id
- self._endTransaction(intermediate=True)
+ self.tm.endTransaction(intermediate=True)
for representative_id in representatives.itervalues():
representative = POTMsgSet.get(representative_id)
self._scrubPOTMsgSetTranslations(representative)
- self._endTransaction(intermediate=True)
+ self.tm.endTransaction(intermediate=True)
- def _mapRepresentatives(self, potemplates):
+ def _mapRepresentatives(self):
"""Map out POTMsgSets' subordinates and templates.
- :param potemplates: An equivalence class of `POTemplate`s to
- sort out.
:return: A tuple of dicts. The first maps each `POTMsgSet`'s
key (as returned by `get_potmsgset_key`) to a list of its
subordinate `POTMsgSet`s. The second maps each
@@ -364,7 +399,7 @@
# key we find, the first POTMsgSet is the representative one.
order_check = OrderingCheck(cmp=self.compare_template_precedence)
- for template in potemplates:
+ for template in self.potemplates:
order_check.check(template)
for potmsgset in template.getPOTMsgSets(False, prefetch=False):
key = get_potmsgset_key(potmsgset)
@@ -379,12 +414,9 @@
return subordinates, representative_templates
- def _mergePOTMsgSets(self, potemplates):
+ def mergePOTMsgSets(self):
"""Merge POTMsgSets for given sequence of sharing templates."""
- self._setUpUtilities()
-
- subordinates, representative_templates = self._mapRepresentatives(
- potemplates)
+ subordinates, representative_templates = self._mapRepresentatives()
num_representatives = len(subordinates)
representative_num = 0
@@ -446,7 +478,7 @@
removeSecurityProxy(subordinate).destroySelf()
potmsgset_deletions += 1
- self._endTransaction(intermediate=True)
+ self.tm.endTransaction(intermediate=True)
report = "Deleted POTMsgSets: %d. TranslationMessages: %d." % (
potmsgset_deletions, tm_deletions)
@@ -455,19 +487,19 @@
else:
log.log(DEBUG2, report)
- def _getPOTMsgSetIds(self, template):
+ @staticmethod
+ def _getPOTMsgSetIds(template):
"""Get list of ids for `template`'s `POTMsgSet`s."""
return [
potmsgset.id
for potmsgset in template.getPOTMsgSets(False, prefetch=False)]
- def _mergeTranslationMessages(self, potemplates):
+ def mergeTranslationMessages(self):
"""Share `TranslationMessage`s between templates where possible."""
- self._setUpUtilities()
order_check = OrderingCheck(cmp=self.compare_template_precedence)
- for template_number, template in enumerate(potemplates):
+ for template_number, template in enumerate(self.potemplates):
log.info("Merging template %d/%d." % (
- template_number + 1, len(potemplates)))
+ template_number + 1, len(self.potemplates)))
deletions = 0
order_check.check(template)
potmsgset_ids = self._getPOTMsgSetIds(template)
@@ -483,7 +515,7 @@
message = TranslationMessage.get(id)
removeSecurityProxy(message).shareIfPossible()
- self._endTransaction(intermediate=True)
+ self.tm.endTransaction(intermediate=True)
after = potmsgset.getAllTranslationMessages().count()
deletions += max(0, before - after)
@@ -494,7 +526,8 @@
else:
log.log(DEBUG2, report)
- def _getPOTMsgSetTranslationMessageKey(self, tm):
+ @staticmethod
+ def _getPOTMsgSetTranslationMessageKey(tm):
"""Return tuple that identifies a TranslationMessage in a POTMsgSet.
A TranslationMessage is identified by (potemplate, potmsgset,
@@ -509,7 +542,8 @@
return (tm.potemplateID, tm.languageID) + msgstr_ids
- def _partitionTranslationMessageIds(self, potmsgset):
+ @staticmethod
+ def _partitionTranslationMessageIds(potmsgset):
"""Partition `TranslationMessage`s by language.
Only the ids are stored, not the `TranslationMessage` objects
@@ -545,7 +579,7 @@
# migration phase can be scrapped.
ids_per_language = self._partitionTranslationMessageIds(potmsgset)
- self._endTransaction(intermediate=True)
+ self.tm.endTransaction(intermediate=True)
deletions = 0
@@ -577,7 +611,7 @@
else:
translations[key] = tm
- self._endTransaction(intermediate=True)
+ self.tm.endTransaction(intermediate=True)
report = "Deleted TranslationMessages: %d" % deletions
if deletions > 0:
@@ -585,7 +619,8 @@
else:
log.log(DEBUG2, report)
- def _findClashes(self, message, target_potmsgset, target_potemplate):
+ @staticmethod
+ def _findClashes(message, target_potmsgset, target_potemplate):
"""What would clash if we moved `message` to the target environment?
A clash can be either `message` being current when the target
@@ -646,7 +681,8 @@
# have to go.
return False
- def _divergeTo(self, message, target_potmsgset, target_potemplate):
+ @classmethod
+ def _divergeTo(cls, message, target_potmsgset, target_potemplate):
"""Attempt to save `message` by diverging to `target_potemplate`.
:param message: a TranslationMessage to save by diverging.
@@ -658,7 +694,7 @@
True, you're done with `message`. If False, you'll have to
find another place for it.
"""
- clashing_current, clashing_imported, twin = self._findClashes(
+ clashing_current, clashing_imported, twin = cls._findClashes(
message, target_potmsgset, target_potemplate)
if clashing_current is not None or clashing_imported is not None:
=== modified file 'scripts/rosetta/message-sharing-merge.py'
--- scripts/rosetta/message-sharing-merge.py 2010-04-27 19:48:39 +0000
+++ scripts/rosetta/message-sharing-merge.py 2011-02-09 16:43:57 +0000
@@ -9,7 +9,7 @@
import _pythonpath
-from lp.translations.scripts.message_sharing_migration import (
+from lp.translations.translationmerger import (
MessageSharingMerge)
# This script merges POTMsgSets for sharing POTemplates. This involves
