launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #14909
[Merge] lp:~stevenk/launchpad/remove-suggested-translation-files into lp:launchpad
Steve Kowalik has proposed merging lp:~stevenk/launchpad/remove-suggested-translation-files into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~stevenk/launchpad/remove-suggested-translation-files/+merge/143058
Destroy ITranslationsPerson.suggestTranslatableFiles() and all of its callsites. This should hopefully stop the timeouts on Person:+translations since we no longer want to do an anti-join involving a few million rows.
--
https://code.launchpad.net/~stevenk/launchpad/remove-suggested-translation-files/+merge/143058
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/remove-suggested-translation-files into lp:launchpad.
=== modified file 'lib/lp/translations/browser/person.py'
--- lib/lp/translations/browser/person.py 2012-10-30 15:54:00 +0000
+++ lib/lp/translations/browser/person.py 2013-01-14 03:08:26 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Person-related translations view classes."""
@@ -333,17 +333,6 @@
return TranslateLinksAggregator().aggregate(pofiles)
- def _suggestTargetsForTranslation(self, max_fetch=None):
- """Suggest translations this person could be helping complete."""
- person = ITranslationsPerson(self.context)
- pofiles = person.suggestTranslatableFiles(
- no_older_than=self.history_horizon)
-
- if max_fetch is not None:
- pofiles = pofiles[:max_fetch]
-
- return TranslateLinksAggregator().aggregate(pofiles)
-
@cachedproperty
def all_projects_and_packages_to_review(self):
"""Top projects and packages for this person to review."""
@@ -427,9 +416,6 @@
list_length)
fetch = 5 * (list_length - len(overall))
- suggestions = self._suggestTargetsForTranslation(fetch)
- overall = self._addToTargetsList(
- overall, suggestions, list_length, list_length)
return overall
=== modified file 'lib/lp/translations/browser/tests/test_persontranslationview.py'
--- lib/lp/translations/browser/tests/test_persontranslationview.py 2012-10-30 15:54:00 +0000
+++ lib/lp/translations/browser/tests/test_persontranslationview.py 2013-01-14 03:08:26 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -308,38 +308,9 @@
nonurgent_pofile.potemplate.productseries.product,
descriptions[0]['target'])
- def test_suggestTargetsForTranslation(self):
- # suggestTargetsForTranslation finds targets that the person
- # could help translate.
- previous_contrib = self._makePOFiles(1, previously_worked_on=True)
- pofile = self._makePOFiles(1, previously_worked_on=False)[0]
- self._addUntranslatedMessages(pofile, 1)
-
- descriptions = self.view._suggestTargetsForTranslation()
-
- self.assertEqual(1, len(descriptions))
- self.assertEqual(
- pofile.potemplate.productseries.product,
- descriptions[0]['target'])
-
- def test_suggestTargetsForTranslation_limits_query(self):
- # The max_fetch argument limits how many POFiles
- # suggestTargetsForTranslation fetches.
- previous_contrib = self._makePOFiles(1, previously_worked_on=True)
- pofiles = self._makePOFiles(3, previously_worked_on=False)
- for pofile in pofiles:
- self._addUntranslatedMessages(pofile, 1)
-
- descriptions = self.view._suggestTargetsForTranslation(max_fetch=2)
-
- self.assertEqual(2, len(descriptions))
- self.assertNotEqual(
- descriptions[0]['target'], descriptions[1]['target'])
-
def test_top_projects_and_packages_to_translate(self):
# top_projects_and_packages_to_translate lists targets that the
- # user has worked on and could help translate, followed by
- # randomly suggested ones that also need translation.
+ # user has worked on and could help translate.
worked_on = self._makePOFiles(1, previously_worked_on=True)[0]
self._addUntranslatedMessages(worked_on, 1)
not_worked_on = self._makePOFiles(1, previously_worked_on=False)[0]
@@ -347,13 +318,10 @@
descriptions = self.view.top_projects_and_packages_to_translate
- self.assertEqual(2, len(descriptions))
+ self.assertEqual(1, len(descriptions))
self.assertEqual(
worked_on.potemplate.productseries.product,
descriptions[0]['target'])
- self.assertEqual(
- not_worked_on.potemplate.productseries.product,
- descriptions[1]['target'])
def test_top_p_n_p_to_translate_caps_existing_involvement(self):
# top_projects_and_packages_to_translate shows no more than 6
@@ -402,7 +370,7 @@
self._addUntranslatedMessages(pofile, 1)
descriptions = self.view.top_projects_and_packages_to_translate
- self.assertEqual(10, len(descriptions))
+ self.assertEqual(6, len(descriptions))
def test_requires_preferred_languages(self):
# requires_preferred_languages tells the page whether this
=== modified file 'lib/lp/translations/interfaces/translationsperson.py'
--- lib/lp/translations/interfaces/translationsperson.py 2012-10-29 06:13:44 +0000
+++ lib/lp/translations/interfaces/translationsperson.py 2013-01-14 03:08:26 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -72,12 +72,3 @@
:return: A query result of `POFile`s ordered by number of
untranslated messages.
"""
-
- def suggestTranslatableFiles(no_older_than=None):
- """Suggest `POFile`s this person could help translate.
-
- Similar to `getTranslatableFiles`, this method picks an
- arbitrary series of `POFile`s that the user is not a reviewer
- for and has not worked on recently, but which are in a language
- the user works in.
- """
=== modified file 'lib/lp/translations/model/translationsperson.py'
--- lib/lp/translations/model/translationsperson.py 2012-10-30 15:54:00 +0000
+++ lib/lp/translations/model/translationsperson.py 2013-01-14 03:08:26 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -161,15 +161,9 @@
'(SELECT * FROM translatable_distroseries)'))).config(
distinct=True).order_by(POFile.date_changed)
- def _queryTranslatableFiles(self, worked_on, no_older_than=None,
- languages=None):
+ def _queryTranslatableFiles(self, no_older_than=None, languages=None):
"""Get `POFile`s this person could help translate.
- :param worked_on: If True, get `POFile`s that the person has
- been working on recently (where "recently" is defined as
- `no_older_than`). If False, get ones that the person has
- not been working on recently (those that the person has
- never worked on, or last worked on before `no_older_than`).
:param no_older_than: Oldest involvement to consider. If the
person last worked on a `POFile` before this date, that
counts as not having worked on it.
@@ -179,18 +173,25 @@
if self.person.is_team:
return []
- tables = self._composePOFileReviewerJoins(
- expect_reviewer_status=False)
-
- translator_join, translator_condition = (
- self._composePOFileTranslatorJoin(worked_on, no_older_than))
+ tables = self._composePOFileReviewerJoins(expect_reviewer_status=False)
+
+ join_condition = And(
+ POFileTranslator.personID == self.person.id,
+ POFileTranslator.pofileID == POFile.id,
+ POFile.language != getUtility(ILaunchpadCelebrities).english)
+
+ if no_older_than is not None:
+ join_condition = And(
+ join_condition,
+ POFileTranslator.date_last_touched >= no_older_than)
+
+ translator_join = Join(POFileTranslator, join_condition)
tables.append(translator_join)
translated_count = (
POFile.currentcount + POFile.updatescount + POFile.rosettacount)
- conditions = And(
- translated_count < POTemplate.messagecount, translator_condition)
+ conditions = translated_count < POTemplate.messagecount
# The person must not be a reviewer for this translation (unless
# it's in the sense that any user gets review permissions
@@ -219,12 +220,11 @@
if languages is not None:
conditions = And(conditions, POFile.languageID.is_in(languages))
- source = Store.of(self.person).using(*tables)
- return source.find(POFile, conditions)
+ return Store.of(self.person).using(*tables).find(POFile, conditions)
def getTranslatableFiles(self, no_older_than=None, urgent_first=True):
"""See `ITranslationsPerson`."""
- results = self._queryTranslatableFiles(True, no_older_than)
+ results = self._queryTranslatableFiles(no_older_than)
translated_count = (
POFile.currentcount + POFile.updatescount + POFile.rosettacount)
@@ -234,16 +234,6 @@
return results.order_by(ordering)
- def suggestTranslatableFiles(self, no_older_than=None):
- """See `ITranslationsPerson`."""
- # XXX JeroenVermeulen 2009-08-28: Ideally this would also check
- # for a free licence. That's hard to do in SQL though.
- languages = set([
- language.id for language in self.translatable_languages])
- results = self._queryTranslatableFiles(
- False, no_older_than, languages=languages)
- return results.order_by(['random()'])
-
def _composePOFileReviewerCTEs(self, no_older_than):
"""Compose Storm CTEs for common `POFile` queries.
@@ -405,42 +395,3 @@
TranslatorJoin,
TranslationTeamJoin,
]
-
- def _composePOFileTranslatorJoin(self, expected_presence,
- no_older_than=None):
- """Compose join condition for `POFileTranslator`.
-
- Checks for a `POFileTranslator` record matching a `POFile` and
- `Person` in a join.
-
- :param expected_presence: whether the `POFileTranslator` record
- is to be present, or absent. The join will enforce presence
- through a regular inner join, or absence by an outer join
- with a condition that the record not be present.
- :param no_older_than: optional cutoff date. `POFileTranslator`
- records older than this date are not considered.
- :return: a tuple of the join, and a condition to be checked by
- the query. Combine it with the query's other conditions
- using `And`.
- """
- join_condition = And(
- POFileTranslator.personID == self.person.id,
- POFileTranslator.pofileID == POFile.id,
- POFile.language != getUtility(ILaunchpadCelebrities).english)
-
- if no_older_than is not None:
- join_condition = And(
- join_condition,
- POFileTranslator.date_last_touched >= no_older_than)
-
- if expected_presence:
- # A regular inner join enforces this. No need for an extra
- # condition; the join does it more efficiently.
- return Join(POFileTranslator, join_condition), True
- else:
- # Check for absence. Usually the best way to check for this
- # is an outer join plus a condition that the outer join
- # match no record.
- return (
- LeftJoin(POFileTranslator, join_condition),
- POFileTranslator.id == None)
=== modified file 'lib/lp/translations/tests/test_potmsgset.py'
--- lib/lp/translations/tests/test_potmsgset.py 2013-01-07 02:40:55 +0000
+++ lib/lp/translations/tests/test_potmsgset.py 2013-01-14 03:08:26 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -408,7 +408,7 @@
self.assertContentEqual(
self.potmsgset.getExternallyUsedTranslationMessages(language),
[other_translation, current_translation])
- self.assertEquals(
+ self.assertContentEqual(
self.potmsgset.getExternallySuggestedOrUsedTranslationMessages(
used_languages=[language])[language].used,
[other_translation, current_translation])
Follow ups
