From 52e28d91fa2d9411e4e2ad4fe612cb47d07c4307 Mon Sep 17 00:00:00 2001 From: Adeel Khan Date: Thu, 15 Jun 2017 01:48:08 +0500 Subject: [PATCH] Implementing connection timeout for sailthru api call. LEARNER-1186 --- ..._user_registration_cookie_timeout_delay.py | 19 +++++++ lms/djangoapps/email_marketing/models.py | 8 +++ lms/djangoapps/email_marketing/signals.py | 52 ++++++++----------- lms/djangoapps/email_marketing/tasks.py | 40 ++++++++++++++ .../email_marketing/tests/test_signals.py | 28 ++++++++-- 5 files changed, 114 insertions(+), 33 deletions(-) create mode 100644 lms/djangoapps/email_marketing/migrations/0005_emailmarketingconfiguration_user_registration_cookie_timeout_delay.py diff --git a/lms/djangoapps/email_marketing/migrations/0005_emailmarketingconfiguration_user_registration_cookie_timeout_delay.py b/lms/djangoapps/email_marketing/migrations/0005_emailmarketingconfiguration_user_registration_cookie_timeout_delay.py new file mode 100644 index 0000000000..a40ba64894 --- /dev/null +++ b/lms/djangoapps/email_marketing/migrations/0005_emailmarketingconfiguration_user_registration_cookie_timeout_delay.py @@ -0,0 +1,19 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('email_marketing', '0004_emailmarketingconfiguration_welcome_email_send_delay'), + ] + + operations = [ + migrations.AddField( + model_name='emailmarketingconfiguration', + name='user_registration_cookie_timeout_delay', + field=models.FloatField(default=1.5, help_text='The number of seconds to delay/timeout wait to get cookie values from sailthru.'), + ), + ] diff --git a/lms/djangoapps/email_marketing/models.py b/lms/djangoapps/email_marketing/models.py index 7e81095865..841b90a867 100644 --- a/lms/djangoapps/email_marketing/models.py +++ b/lms/djangoapps/email_marketing/models.py @@ -136,6 +136,14 @@ class EmailMarketingConfiguration(ConfigurationModel): ) ) + # The number of seconds to delay/timeout wait to get cookie values from sailthru. + user_registration_cookie_timeout_delay = models.fields.FloatField( + default=1.5, + help_text=_( + "The number of seconds to delay/timeout wait to get cookie values from sailthru." + ) + ) + def __unicode__(self): return u"Email marketing configuration: New user list %s, Activation template: %s" % \ (self.sailthru_new_user_list, self.sailthru_activation_template) diff --git a/lms/djangoapps/email_marketing/signals.py b/lms/djangoapps/email_marketing/signals.py index e83f57eccb..b5fa747a4c 100644 --- a/lms/djangoapps/email_marketing/signals.py +++ b/lms/djangoapps/email_marketing/signals.py @@ -9,9 +9,10 @@ from django.conf import settings from django.dispatch import receiver from sailthru.sailthru_client import SailthruClient from sailthru.sailthru_error import SailthruClientError +from celery.exceptions import TimeoutError from email_marketing.models import EmailMarketingConfiguration -from lms.djangoapps.email_marketing.tasks import update_user, update_user_email +from lms.djangoapps.email_marketing.tasks import update_user, update_user_email, get_email_cookies_via_sailthru from student.cookies import CREATE_LOGON_COOKIE from student.views import REGISTER_USER from util.model_utils import USER_FIELD_CHANGED @@ -54,41 +55,34 @@ def add_email_marketing_cookies(sender, response=None, user=None, if sailthru_content: post_parms['cookies'] = {'anonymous_interest': sailthru_content} + time_before_call = datetime.datetime.now() + sailthru_response = get_email_cookies_via_sailthru.delay(user.email, post_parms) + try: - sailthru_client = SailthruClient(email_config.sailthru_key, email_config.sailthru_secret) - log.info( - 'Sending to Sailthru the user interest cookie [%s] for user [%s]', - post_parms.get('cookies', ''), - user.email - ) - time_before_call = datetime.datetime.now() - - sailthru_response = \ - sailthru_client.api_post("user", post_parms) + # synchronous call to get result of an asynchronous celery task, with timeout + sailthru_response.get(timeout=email_config.user_registration_cookie_timeout_delay, + propagate=True) + cookie = sailthru_response.result + except TimeoutError as exc: + log.error("Timeout error while attempting to obtain cookie from Sailthru: %s", unicode(exc)) + return response except SailthruClientError as exc: log.error("Exception attempting to obtain cookie from Sailthru: %s", unicode(exc)) - return response - if sailthru_response.is_ok(): - if 'keys' in sailthru_response.json and 'cookie' in sailthru_response.json['keys']: - cookie = sailthru_response.json['keys']['cookie'] - - response.set_cookie( - 'sailthru_hid', - cookie, - max_age=365 * 24 * 60 * 60, # set for 1 year - domain=settings.SESSION_COOKIE_DOMAIN, - path='/', - ) - _log_sailthru_api_call_time(time_before_call) - else: - log.error("No cookie returned attempting to obtain cookie from Sailthru for %s", user.email) + if not cookie: + log.error("No cookie returned attempting to obtain cookie from Sailthru for %s", user.email) + return response else: - error = sailthru_response.get_error() - # generally invalid email address - log.info("Error attempting to obtain cookie from Sailthru: %s", error.get_message()) + response.set_cookie( + 'sailthru_hid', + cookie, + max_age=365 * 24 * 60 * 60, # set for 1 year + domain=settings.SESSION_COOKIE_DOMAIN, + path='/', + ) + _log_sailthru_api_call_time(time_before_call) return response diff --git a/lms/djangoapps/email_marketing/tasks.py b/lms/djangoapps/email_marketing/tasks.py index 95955475e2..a6e952e515 100644 --- a/lms/djangoapps/email_marketing/tasks.py +++ b/lms/djangoapps/email_marketing/tasks.py @@ -17,6 +17,46 @@ log = logging.getLogger(__name__) SAILTHRU_LIST_CACHE_KEY = "email.marketing.cache" +@task(bind=True) +def get_email_cookies_via_sailthru(self, user_email, post_parms): + """ + Adds/updates Sailthru cookie information for a new user. + Args: + post_parms(dict): User profile information to pass as 'vars' to Sailthru + Returns: + cookie(str): cookie fetched from Sailthru + """ + + email_config = EmailMarketingConfiguration.current() + if not email_config.enabled: + return None + + try: + sailthru_client = SailthruClient(email_config.sailthru_key, email_config.sailthru_secret) + log.info( + 'Sending to Sailthru the user interest cookie [%s] for user [%s]', + post_parms.get('cookies', ''), + user_email + ) + sailthru_response = sailthru_client.api_post("user", post_parms) + except SailthruClientError as exc: + log.error("Exception attempting to obtain cookie from Sailthru: %s", unicode(exc)) + raise SailthruClientError + + if sailthru_response.is_ok(): + if 'keys' in sailthru_response.json and 'cookie' in sailthru_response.json['keys']: + cookie = sailthru_response.json['keys']['cookie'] + return cookie + else: + log.error("No cookie returned attempting to obtain cookie from Sailthru for %s", user_email) + else: + error = sailthru_response.get_error() + # generally invalid email address + log.info("Error attempting to obtain cookie from Sailthru: %s", error.get_message()) + + return None + + # pylint: disable=not-callable @task(bind=True, default_retry_delay=3600, max_retries=24) def update_user(self, sailthru_vars, email, site=None, new_user=False, activation=False): diff --git a/lms/djangoapps/email_marketing/tests/test_signals.py b/lms/djangoapps/email_marketing/tests/test_signals.py index 1d12ac0aa7..6faece5c64 100644 --- a/lms/djangoapps/email_marketing/tests/test_signals.py +++ b/lms/djangoapps/email_marketing/tests/test_signals.py @@ -25,7 +25,8 @@ from email_marketing.tasks import ( _get_list_from_email_marketing_provider, _get_or_create_user_list, update_user, - update_user_email + update_user_email, + get_email_cookies_via_sailthru ) from student.tests.factories import UserFactory, UserProfileFactory from util.json_request import JsonResponse @@ -98,13 +99,10 @@ class EmailMarketingTests(TestCase): self.request.COOKIES['anonymous_interest'] = 'cookie_content' mock_get_current_request.return_value = self.request mock_sailthru.return_value = SailthruResponse(JsonResponse({'keys': {'cookie': 'test_cookie'}})) - cookie_log = "Sending to Sailthru the user interest cookie [{'anonymous_interest': 'cookie_content'}]" \ - ' for user [test@edx.org]' with LogCapture(LOGGER_NAME, level=logging.INFO) as logger: add_email_marketing_cookies(None, response=response, user=self.user) logger.check( - (LOGGER_NAME, 'INFO', cookie_log), (LOGGER_NAME, 'INFO', 'Started at {start} and ended at {end}, time spent:{delta} milliseconds'.format( start=datetime.datetime.now().isoformat(' '), @@ -120,6 +118,28 @@ class EmailMarketingTests(TestCase): self.assertTrue('sailthru_hid' in response.cookies) self.assertEquals(response.cookies['sailthru_hid'].value, "test_cookie") + @patch('email_marketing.signals.SailthruClient.api_post') + def test_get_cookies_via_sailthu(self, mock_sailthru): + + cookies = {'cookie': 'test_cookie'} + mock_sailthru.return_value = SailthruResponse(JsonResponse({'keys': cookies})) + + post_parms = { + 'id': self.user.email, + 'fields': {'keys': 1}, + 'vars': {'last_login_date': datetime.datetime.now().strftime("%Y-%m-%d")}, + 'cookies': {'anonymous_interest': 'cookie_content'} + } + expected_cookie = get_email_cookies_via_sailthru.delay(self.user.email, post_parms) + + mock_sailthru.assert_called_with('user', + {'fields': {'keys': 1}, + 'cookies': {'anonymous_interest': 'cookie_content'}, + 'id': TEST_EMAIL, + 'vars': {'last_login_date': ANY}}) + + self.assertEqual(cookies['cookie'], expected_cookie.result) + @patch('email_marketing.signals.SailthruClient.api_post') def test_drop_cookie_error_path(self, mock_sailthru): """