launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #05332
[Merge] lp:~benji/launchpad/bug-877195-code into lp:launchpad
Benji York has proposed merging lp:~benji/launchpad/bug-877195-code into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #877195 in Launchpad itself: "Move statistics updating outside the web browser code"
https://bugs.launchpad.net/launchpad/+bug/877195
For more details, see:
https://code.launchpad.net/~benji/launchpad/bug-877195-code/+merge/80617
When a change is made to a translation several statistics about that
translation are updated. That update is slow. So slow that it often
times out. Bug 877195 is about moving those statistics updates into a
job so the web requests can return faster but the statistics will still
be relatively up to date.
I had pre-implementation calls with Danilo.
There is a prerequisite branch that includes the database changes.
Tests for the new job are in
lib/lp/translations/tests/test_pofilestatsjob.py and can be run like so:
bin/test -c -m lp.translations.tests.test_pofilestatsjob
The "make lint" report is clean.
QA: note a translations statistics, make a change to a translation that
would impact its statistics, get a LOSA to run the cron job, verify that
the statistics changed.
--
https://code.launchpad.net/~benji/launchpad/bug-877195-code/+merge/80617
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~benji/launchpad/bug-877195-code into lp:launchpad.
=== modified file 'database/schema/comments.sql'
--- database/schema/comments.sql 2011-10-07 11:32:23 +0000
+++ database/schema/comments.sql 2011-10-27 20:30:31 +0000
@@ -2390,15 +2390,18 @@
COMMENT ON TABLE DatabaseCpuStats IS 'Snapshots of CPU utilization per database username.';
COMMENT ON COLUMN DatabaseCpuStats.cpu IS '% CPU utilization * 100, as reported by ps -o cp';
-
-- SuggestivePOTemplate
COMMENT ON TABLE SuggestivePOTemplate IS
'Cache of POTemplates that can provide external translation suggestions.';
-
-- OpenIdIdentifier
COMMENT ON TABLE OpenIdIdentifier IS
'OpenId Identifiers that can be used to log into an Account.';
COMMENT ON COLUMN OpenIdIdentifier.identifier IS
'OpenId Identifier. This should be a URL, but is currently just a token that can be used to generate the Identity URL for the Canonical SSO OpenId Provider.';
+-- POFileStatsJob
+COMMENT ON TABLE POFileStatsJob IS
+'Scheduled jobs that are to update POFile statistics.';
+COMMENT ON COLUMN POFileStatsJob.job IS 'A reference to a row in the Job table that has all the common job details.';
+COMMENT ON COLUMN POFileStatsJob.pofile IS 'The POFile that this job is for.';
=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg 2011-10-20 08:52:37 +0000
+++ database/schema/security.cfg 2011-10-27 20:30:31 +0000
@@ -247,6 +247,7 @@
public.persontransferjob = SELECT, INSERT, UPDATE, DELETE
public.pillarname = SELECT, INSERT, DELETE
public.poexportrequest = SELECT, INSERT, UPDATE, DELETE
+public.pofilestatsjob = SELECT, INSERT, UPDATE, DELETE
public.pofiletranslator = SELECT
public.poll = SELECT, INSERT, UPDATE
public.polloption = SELECT, INSERT, UPDATE, DELETE
@@ -438,6 +439,8 @@
public.language = SELECT
public.pofile = SELECT, UPDATE
public.potemplate = SELECT, UPDATE
+public.job = SELECT, UPDATE, DELETE
+public.pofilestatsjob = SELECT, UPDATE, DELETE
public.potmsgset = SELECT
public.translationmessage = SELECT
public.translationtemplateitem = SELECT
=== modified file 'lib/canonical/config/schema-lazr.conf'
--- lib/canonical/config/schema-lazr.conf 2011-10-19 19:14:36 +0000
+++ lib/canonical/config/schema-lazr.conf 2011-10-27 20:30:31 +0000
@@ -1834,6 +1834,13 @@
dbuser: send-branch-mail
storm_cache: generational
storm_cache_size: 500
+oops_prefix: none
+error_dir: none
+
+[pofile_stats]
+dbuser: pofilestats
+storm_cache: generational
+source_interface: lp.translations.interfaces.pofilestatsjob.IPOFileStatsJobSource
# See [error_reports].
error_dir: none
=== modified file 'lib/lp/translations/configure.zcml'
--- lib/lp/translations/configure.zcml 2011-10-18 10:09:58 +0000
+++ lib/lp/translations/configure.zcml 2011-10-27 20:30:31 +0000
@@ -694,4 +694,13 @@
<webservice:register module="lp.translations.interfaces.webservice" />
+ <class
+ class="lp.translations.model.pofilestatsjob.POFileStatsJob">
+ <allow interface='lp.services.job.interfaces.job.IRunnableJob'/>
+ </class>
+ <utility
+ component="lp.translations.model.pofilestatsjob.POFileStatsJob"
+ provides="lp.translations.interfaces.pofilestatsjob.IPOFileStatsJobSource"
+ />
+
</configure>
=== added file 'lib/lp/translations/interfaces/pofilestatsjob.py'
--- lib/lp/translations/interfaces/pofilestatsjob.py 1970-01-01 00:00:00 +0000
+++ lib/lp/translations/interfaces/pofilestatsjob.py 2011-10-27 20:30:31 +0000
@@ -0,0 +1,16 @@
+# Copyright 2010 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+# pylint: disable-msg=E0213
+
+__metaclass__ = type
+
+__all__ = [
+ 'IPOFileStatsJobSource',
+ ]
+
+from lp.services.job.interfaces.job import IJobSource
+
+
+class IPOFileStatsJobSource(IJobSource):
+ """A source for jobs to update POFile statistics."""
=== added file 'lib/lp/translations/model/pofilestatsjob.py'
--- lib/lp/translations/model/pofilestatsjob.py 1970-01-01 00:00:00 +0000
+++ lib/lp/translations/model/pofilestatsjob.py 2011-10-27 20:30:31 +0000
@@ -0,0 +1,103 @@
+# Copyright 2011 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Job for merging translations."""
+
+
+__metaclass__ = type
+
+
+__all__ = [
+ 'POFileStatsJob',
+ ]
+
+import logging
+from zope.component import getUtility
+
+from storm.locals import (
+ And,
+ Int,
+ Reference,
+ )
+from zope.interface import (
+ classProvides,
+ implements,
+ )
+
+from canonical.launchpad.webapp.interfaces import (
+ DEFAULT_FLAVOR,
+ IStoreSelector,
+ MAIN_STORE,
+ )
+from lp.services.database.stormbase import StormBase
+from lp.services.job.interfaces.job import IRunnableJob
+from lp.services.job.model.job import Job
+from lp.services.job.runner import BaseRunnableJob
+from lp.translations.interfaces.pofilestatsjob import IPOFileStatsJobSource
+from lp.translations.model.pofile import POFile
+
+
+class POFileStatsJob(StormBase, BaseRunnableJob):
+ """The details for a POFile status update job."""
+
+ __storm_table__ = 'POFileStatsJob'
+
+ # Instances of this class are runnable jobs.
+ implements(IRunnableJob)
+
+ # Oddly, BaseRunnableJob inherits from BaseRunnableJobSource so this class
+ # is both the factory for jobs (the "implements", above) and the source
+ # for runnable jobs (not the constructor of the job source, the class
+ # provides the IJobSource interface itself).
+ classProvides(IPOFileStatsJobSource)
+
+ id = Int(primary=True)
+
+ # The Job table contains core job details.
+ job_id = Int('job')
+ job = Reference(job_id, Job.id)
+
+ # This is the POFile which needs its statistics updated.
+ pofile_id = Int('pofile')
+ pofile = Reference(pofile_id, POFile.id)
+
+ def __init__(self, pofile):
+ self.job = Job()
+ self.pofile = pofile
+ super(POFileStatsJob, self).__init__()
+
+ def getOperationDescription(self):
+ """See `IRunnableJob`."""
+ return 'updating POFile statistics'
+
+ def run(self):
+ """See `IRunnableJob`."""
+ logger = logging.getLogger()
+ logger.info('Updating statistics for %s' % self.pofile.title)
+ self.pofile.updateStatistics()
+
+ @staticmethod
+ def iterReady():
+ """See `IJobSource`."""
+ store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
+ return store.find((POFileStatsJob),
+ And(POFileStatsJob.job == Job.id,
+ Job.id.is_in(Job.ready_jobs)))
+
+
+def schedule(pofile):
+ """Schedule a job to update a POFile's stats (if not scheduled)."""
+ # If there's already a job for the pofile, don't create a new one.
+ store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
+ job = store.find(
+ POFileStatsJob,
+ POFileStatsJob.pofile == pofile,
+ POFileStatsJob.job == Job.id,
+ Job.id.is_in(Job.ready_jobs)
+ ).any()
+
+ if job is None:
+ job = POFileStatsJob(pofile)
+
+ return job
+
=== added file 'lib/lp/translations/tests/test_pofilestatsjob.py'
--- lib/lp/translations/tests/test_pofilestatsjob.py 1970-01-01 00:00:00 +0000
+++ lib/lp/translations/tests/test_pofilestatsjob.py 2011-10-27 20:30:31 +0000
@@ -0,0 +1,76 @@
+# Copyright 2011 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for merging translations."""
+
+__metaclass__ = type
+
+
+from canonical.launchpad.webapp.testing import verifyObject
+from canonical.testing.layers import (
+ LaunchpadZopelessLayer,
+ )
+from lp.registry.interfaces.packaging import IPackagingUtil
+from lp.services.job.interfaces.job import (
+ IJobSource,
+ IRunnableJob,
+ )
+from lp.testing import (
+ person_logged_in,
+ TestCaseWithFactory,
+ )
+
+from lp.translations.interfaces.pofilestatsjob import IPOFileStatsJobSource
+from lp.translations.interfaces.side import TranslationSide
+from lp.translations.model import pofilestatsjob
+from lp.translations.model.pofilestatsjob import POFileStatsJob
+
+
+def runable_jobs():
+ """Returns a list of the currently runnable stats update jobs.
+
+ Provides a nicer spelling for tests than the crazy static method on the
+ job class.
+
+ The return value is listified because that's what we want for tests.
+ """
+ return list(POFileStatsJob.iterReady())
+
+
+class TestPOFileStatsJob(TestCaseWithFactory):
+
+ layer = LaunchpadZopelessLayer
+
+ def test_job_interface(self):
+ # Instances of POFileStatsJob are runnable jobs.
+ verifyObject(IRunnableJob, POFileStatsJob(0))
+
+ def test_source_interface(self):
+ # The POFileStatsJob class is a source of POFileStatsJobs.
+ verifyObject(IPOFileStatsJobSource, POFileStatsJob)
+ verifyObject(IJobSource, POFileStatsJob)
+
+ def test_run(self):
+ # Running a job causes the POFile statistics to be updated.
+ singular = self.factory.getUniqueString()
+ pofile = self.factory.makePOFile(side=TranslationSide.UPSTREAM)
+ # Create a message so we have something to have statistics about.
+ self.factory.makePOTMsgSet(pofile.potemplate, singular)
+ # The statistics start at 0.
+ self.assertEqual(pofile.potemplate.messageCount(), 0)
+ job = pofilestatsjob.schedule(pofile.id)
+ # Just scheduling the job doesn't update the statistics.
+ self.assertEqual(pofile.potemplate.messageCount(), 0)
+ job.run()
+ # Now that the job ran, the statistics have been updated.
+ self.assertEqual(pofile.potemplate.messageCount(), 1)
+
+ def test_iterReady(self):
+ # The POFileStatsJob class provides a way to iterate over the jobs
+ # that are ready to run. Initially, there aren't any.
+ self.assertEqual(len(list(POFileStatsJob.iterReady())), 0)
+ # We need a POFile to update.
+ pofile = self.factory.makePOFile(side=TranslationSide.UPSTREAM)
+ # If we schedule a job, then we'll get it back.
+ job = pofilestatsjob.schedule(pofile.id)
+ self.assertIs(list(POFileStatsJob.iterReady())[0], job)
