From 5557d95d2e4565741cbf68cd2a8cd9d4c4511223 Mon Sep 17 00:00:00 2001 From: Rob Hudson Date: Tue, 11 Feb 2014 15:51:27 -0800 Subject: [PATCH] Fixed problem with weekly installs calculation (bug 962174) --- mkt/webapps/tasks.py | 48 ++++++++++++++++-------- mkt/webapps/tests/test_crons.py | 65 +++++++++++++++++---------------- scripts/crontab/crontab.tpl | 2 +- 3 files changed, 66 insertions(+), 49 deletions(-) diff --git a/mkt/webapps/tasks.py b/mkt/webapps/tasks.py index 7f639cebb7..9cfc35c113 100644 --- a/mkt/webapps/tasks.py +++ b/mkt/webapps/tasks.py @@ -10,7 +10,6 @@ import time from django.conf import settings from django.contrib.auth.models import AnonymousUser from django.core.files.storage import default_storage as storage -from django.forms import ValidationError from django.template import Context, loader import pytz @@ -657,37 +656,54 @@ def update_trending(ids, **kw): @write def update_downloads(ids, **kw): client = get_monolith_client() - today = datetime.date.today() count = 0 for app in Webapp.objects.filter(id__in=ids).no_transforms(): - kwargs = {'app-id': app.id} + appid = {'app-id': app.id} # Get weekly downloads. - # - # If we query monolith with interval=week and the past 7 days - # crosses a Monday, Monolith splits the counts into two. We want - # the sum over the past week so we need to `sum` these. + query = { + 'query': {'match_all': {}}, + 'facets': { + 'installs': { + 'date_histogram': { + 'value_field': 'app_installs', + 'interval': 'week', + 'key_field': 'date', + }, + 'facet_filter': { + 'and': [ + {'term': appid}, + {'range': {'date': { + 'gte': days_ago(8).date().strftime('%Y-%m-%d'), + 'lte': days_ago(1).date().strftime('%Y-%m-%d'), + }}} + ] + } + } + }, + 'size': 0} + try: + resp = client.raw(query) + # If we query monolith with interval=week and the past 7 days + # crosses a Monday, Monolith splits the counts into two. We want + # the sum over the past week so we need to `sum` these. weekly = sum( - c['count'] for c in - client('app_installs', days_ago(7).date(), today, 'week', - **kwargs) - if c.get('count')) - except ValueError as e: + c['total'] for c in + resp.get('facets', {}).get('installs', {}).get('entries') + if c.get('total')) + except Exception as e: task_log.info('Call to ES failed: {0}'.format(e)) weekly = 0 # Get total downloads. - # - # The monolith client lib doesn't handle this for us so we send a raw - # ES query to Monolith. query = {'query': {'match_all': {}}, 'facets': { 'installs': { 'statistical': {'field': 'app_installs'}, - 'facet_filter': {'term': kwargs}}}, + 'facet_filter': {'term': appid}}}, 'size': 0} try: resp = client.raw(query) diff --git a/mkt/webapps/tests/test_crons.py b/mkt/webapps/tests/test_crons.py index bc906cd2fe..deed96afe8 100644 --- a/mkt/webapps/tests/test_crons.py +++ b/mkt/webapps/tests/test_crons.py @@ -30,53 +30,54 @@ class TestWeeklyDownloads(amo.tests.TestCase): def get_app(self): return Webapp.objects.get(pk=self.app.pk) - def get_mocks(self): - """ - Returns 2 mocks as a tuple. - - First is the `client(...)` call to get histogram data for last 7 days. - Second is the `client.raw(...)` call to get the totals data. - """ - weekly = [ - {'count': 122.0, 'date': date(2013, 12, 30)}, - {'count': 133.0, 'date': date(2014, 1, 6)}, - ] + @mock.patch('mkt.webapps.tasks.get_monolith_client') + def test_weekly_downloads(self, _mock): + client = mock.Mock() raw = { 'facets': { 'installs': { - u'_type': u'statistical', - u'count': 49, - u'max': 224.0, - u'mean': 135.46938775510205, - u'min': 1.0, - u'std_deviation': 67.41619197523309, - u'sum_of_squares': 1121948.0, - u'total': 6638.0, - u'variance': 4544.9429404414832 + '_type': 'date_histogram', + 'entries': [ + {'count': 3, + 'time': 1390780800000, + 'total': 19.0}, + {'count': 62, + 'time': 1391385600000, + 'total': 236.0} + ] } - }, - u'hits': { - u'hits': [], u'max_score': 1.0, u'total': 46957 } } - - return (weekly, raw) - - @mock.patch('mkt.webapps.tasks.get_monolith_client') - def test_weekly_downloads(self, _mock): - weekly, raw = self.get_mocks() - client = mock.Mock() - client.return_value = weekly client.raw.return_value = raw _mock.return_value = client eq_(self.app.weekly_downloads, 0) - eq_(self.app.total_downloads, 0) update_downloads([self.app.pk]) self.app.reload() eq_(self.app.weekly_downloads, 255) + + @mock.patch('mkt.webapps.tasks.get_monolith_client') + def test_total_downloads(self, _mock): + client = mock.Mock() + raw = { + 'facets': { + 'installs': { + u'_type': u'statistical', + u'count': 49, + u'total': 6638.0 + } + } + } + client.raw.return_value = raw + _mock.return_value = client + + eq_(self.app.total_downloads, 0) + + update_downloads([self.app.pk]) + + self.app.reload() eq_(self.app.total_downloads, 6638) @mock.patch('mkt.webapps.tasks.get_monolith_client') diff --git a/scripts/crontab/crontab.tpl b/scripts/crontab/crontab.tpl index a0dd921eac..b841bdb460 100644 --- a/scripts/crontab/crontab.tpl +++ b/scripts/crontab/crontab.tpl @@ -41,6 +41,7 @@ HOME=/tmp 10 8 * * * %(z_cron)s update_monolith_stats `/bin/date -d 'yesterday' +\%%Y-\%%m-\%%d` 15 8 * * * %(z_cron)s process_iarc_changes --settings=settings_local_mkt 30 8 * * * %(z_cron)s dump_user_installs_cron --settings=settings_local_mkt +00 9 * * * %(z_cron)s update_app_downloads --settings=settings_local_mkt 30 9 * * * %(z_cron)s update_user_ratings 50 9 * * * %(z_cron)s gc 45 9 * * * %(z_cron)s mkt_gc --settings=settings_local_mkt @@ -64,7 +65,6 @@ HOME=/tmp #Once per day after 2100 PST (after metrics is done) 35 5 * * * %(z_cron)s update_addon_download_totals 40 5 * * * %(z_cron)s weekly_downloads -40 5 * * * %(z_cron)s update_app_downloads --settings=settings_local_mkt 35 6 * * * %(z_cron)s update_global_totals 40 6 * * * %(z_cron)s update_addon_average_daily_users 30 7 * * * %(z_cron)s index_latest_stats