From c2526844e9d66774526a16e43fff0f0c1981d9bd Mon Sep 17 00:00:00 2001 From: Alex Vandiver Date: Tue, 12 Jan 2021 00:57:39 -0800 Subject: [PATCH] worker: Remove SignupWorker and friends. ZULIP_FRIENDS_LIST_ID and MAILCHIMP_API_KEY are not currently used in production. This removes the unused 'signups' queue and worker. --- puppet/zulip/manifests/app_frontend_base.pp | 1 - .../files/nagios3/conf.d/services.cfg | 11 --- scripts/lib/check_rabbitmq_queue.py | 1 - .../commands/add_users_to_mailing_list.py | 66 -------------- zerver/tests/test_queue_worker.py | 85 ------------------- zerver/worker/queue_processors.py | 27 ------ zproject/computed_settings.py | 2 - 7 files changed, 193 deletions(-) delete mode 100644 zerver/management/commands/add_users_to_mailing_list.py diff --git a/puppet/zulip/manifests/app_frontend_base.pp b/puppet/zulip/manifests/app_frontend_base.pp index 0c550ea116..f2ac35bcfd 100644 --- a/puppet/zulip/manifests/app_frontend_base.pp +++ b/puppet/zulip/manifests/app_frontend_base.pp @@ -77,7 +77,6 @@ class zulip::app_frontend_base { 'missedmessage_emails', 'missedmessage_mobile_notifications', 'outgoing_webhooks', - 'signups', 'user_activity', 'user_activity_interval', 'user_presence', diff --git a/puppet/zulip_ops/files/nagios3/conf.d/services.cfg b/puppet/zulip_ops/files/nagios3/conf.d/services.cfg index f71720e14e..2f5c4dfcfb 100644 --- a/puppet/zulip_ops/files/nagios3/conf.d/services.cfg +++ b/puppet/zulip_ops/files/nagios3/conf.d/services.cfg @@ -349,17 +349,6 @@ define service { contact_groups admins } -define service { - use generic-service - service_description Check RabbitMQ signups consumers - check_command check_rabbitmq_consumers!signups - # Workaround weird checks 40s after first error causing alerts - # from a single failure because cron hasn't run again yet - max_check_attempts 3 - hostgroup_name frontends - contact_groups admins -} - define service { use generic-service service_description Check RabbitMQ digest email consumers diff --git a/scripts/lib/check_rabbitmq_queue.py b/scripts/lib/check_rabbitmq_queue.py index 1a9751218d..45a5598dae 100644 --- a/scripts/lib/check_rabbitmq_queue.py +++ b/scripts/lib/check_rabbitmq_queue.py @@ -20,7 +20,6 @@ normal_queues = [ 'missedmessage_emails', 'missedmessage_mobile_notifications', 'outgoing_webhooks', - 'signups', 'user_activity', 'user_activity_interval', 'user_presence', diff --git a/zerver/management/commands/add_users_to_mailing_list.py b/zerver/management/commands/add_users_to_mailing_list.py deleted file mode 100644 index bdb97ca014..0000000000 --- a/zerver/management/commands/add_users_to_mailing_list.py +++ /dev/null @@ -1,66 +0,0 @@ -import argparse -from datetime import datetime -from typing import Any, Optional - -import orjson -import requests -from django.conf import settings -from django.core.management.base import BaseCommand, CommandError -from django.utils.timezone import now as timezone_now - -from zerver.models import UserProfile - - -class Command(BaseCommand): - help = """Add users to a MailChimp mailing list.""" - - def add_arguments(self, parser: argparse.ArgumentParser) -> None: - parser.add_argument('--api-key', - help='MailChimp API key.') - parser.add_argument('--list-id', - help='List ID of the MailChimp mailing list.') - parser.add_argument('--optin-time', - default=datetime.isoformat(timezone_now().replace(microsecond=0)), - help='Opt-in time of the users.') - - def handle(self, *args: Any, **options: Optional[str]) -> None: - api_key = options['api_key'] - if api_key is None: - try: - if settings.MAILCHIMP_API_KEY is None: - raise CommandError('MAILCHIMP_API_KEY is None. Check your server settings file.') - api_key = settings.MAILCHIMP_API_KEY - except AttributeError: - raise CommandError('Please supply a MailChimp API key to --api-key, or add a ' - 'MAILCHIMP_API_KEY to your server settings file.') - - if options['list_id'] is None: - try: - if settings.ZULIP_FRIENDS_LIST_ID is None: - raise CommandError('ZULIP_FRIENDS_LIST_ID is None. Check your server settings file.') - options['list_id'] = settings.ZULIP_FRIENDS_LIST_ID - except AttributeError: - raise CommandError('Please supply a MailChimp List ID to --list-id, or add a ' - 'ZULIP_FRIENDS_LIST_ID to your server settings file.') - - endpoint = "https://{}.api.mailchimp.com/3.0/lists/{}/members".format( - api_key.split('-')[1], options['list_id'], - ) - - for user in UserProfile.objects.filter(is_bot=False, is_active=True) \ - .values('email', 'full_name', 'realm_id'): - data = { - 'email_address': user['email'], - 'list_id': options['list_id'], - 'status': 'subscribed', - 'merge_fields': { - 'NAME': user['full_name'], - 'REALM_ID': user['realm_id'], - 'OPTIN_TIME': options['optin_time'], - }, - } - r = requests.post(endpoint, auth=('apikey', api_key), json=data, timeout=10) - if r.status_code == 400 and orjson.loads(r.content)['title'] == 'Member Exists': - print("{} is already a part of the list.".format(data['email_address'])) - elif r.status_code >= 400: - print(r.text) diff --git a/zerver/tests/test_queue_worker.py b/zerver/tests/test_queue_worker.py index db7519a312..00e8aec388 100644 --- a/zerver/tests/test_queue_worker.py +++ b/zerver/tests/test_queue_worker.py @@ -402,91 +402,6 @@ class WorkerTest(ZulipTestCase): self.assertEqual(data['failed_tries'], 1 + MAX_REQUEST_RETRIES) - def test_signups_worker_retries(self) -> None: - """Tests the retry logic of signups queue.""" - fake_client = self.FakeClient() - - user_id = self.example_user('hamlet').id - data = {'user_id': user_id, 'id': 'test_missed'} - fake_client.enqueue('signups', data) - - def fake_publish(queue_name: str, event: Dict[str, Any], processor: Optional[Callable[[Any], None]]) -> None: - fake_client.enqueue(queue_name, event) - - fake_response = MagicMock() - fake_response.status_code = 400 - fake_response.content = orjson.dumps({'title': ''}) - with simulated_queue_client(lambda: fake_client): - worker = queue_processors.SignupWorker() - worker.setup() - with patch('zerver.worker.queue_processors.requests.post', - return_value=fake_response), \ - mock_queue_publish('zerver.lib.queue.queue_json_publish', - side_effect=fake_publish), \ - patch('logging.info'), \ - self.settings(MAILCHIMP_API_KEY='one-two', - PRODUCTION=True, - ZULIP_FRIENDS_LIST_ID='id'): - worker.start() - - self.assertEqual(data['failed_tries'], 1 + MAX_REQUEST_RETRIES) - - def test_signups_worker_existing_member(self) -> None: - fake_client = self.FakeClient() - - user_id = self.example_user('hamlet').id - data = {'user_id': user_id, - 'id': 'test_missed', - 'email_address': 'foo@bar.baz'} - fake_client.enqueue('signups', data) - - fake_response = MagicMock() - fake_response.status_code = 400 - fake_response.content = orjson.dumps({'title': 'Member Exists'}) - with simulated_queue_client(lambda: fake_client): - worker = queue_processors.SignupWorker() - worker.setup() - with patch('zerver.worker.queue_processors.requests.post', - return_value=fake_response), \ - self.settings(MAILCHIMP_API_KEY='one-two', - PRODUCTION=True, - ZULIP_FRIENDS_LIST_ID='id'), \ - self.assertLogs(level='INFO') as info_logs: - with patch('logging.warning') as logging_warning_mock: - worker.start() - logging_warning_mock.assert_called_once_with( - "Attempted to sign up already existing email to list: %s", - "foo@bar.baz", - ) - self.assertEqual(info_logs.output, [ - 'INFO:root:Processing signup for user 10 in realm zulip' - ]) - - def test_signups_bad_request(self) -> None: - fake_client = self.FakeClient() - - user_id = self.example_user('hamlet').id - data = {'user_id': user_id, 'id': 'test_missed'} - fake_client.enqueue('signups', data) - - fake_response = MagicMock() - fake_response.status_code = 444 # Any non-400 bad request code. - fake_response.content = orjson.dumps({'title': 'Member Exists'}) - with simulated_queue_client(lambda: fake_client): - worker = queue_processors.SignupWorker() - worker.setup() - with patch('zerver.worker.queue_processors.requests.post', - return_value=fake_response), \ - self.settings(MAILCHIMP_API_KEY='one-two', - PRODUCTION=True, - ZULIP_FRIENDS_LIST_ID='id'), \ - self.assertLogs(level='INFO') as info_logs: - worker.start() - fake_response.raise_for_status.assert_called_once() - self.assertEqual(info_logs.output, [ - 'INFO:root:Processing signup for user 10 in realm zulip' - ]) - def test_invites_worker(self) -> None: fake_client = self.FakeClient() inviter = self.example_user('iago') diff --git a/zerver/worker/queue_processors.py b/zerver/worker/queue_processors.py index 71b3ff4803..ea03a72dfd 100644 --- a/zerver/worker/queue_processors.py +++ b/zerver/worker/queue_processors.py @@ -35,7 +35,6 @@ from typing import ( ) import orjson -import requests import sentry_sdk from django.conf import settings from django.db import connection @@ -370,32 +369,6 @@ class LoopQueueProcessingWorker(QueueProcessingWorker): """In LoopQueueProcessingWorker, consume is used just for automated tests""" self.consume_batch([event]) -@assign_queue('signups') -class SignupWorker(QueueProcessingWorker): - def consume(self, data: Dict[str, Any]) -> None: - # TODO: This is the only implementation with Dict cf Mapping; should we simplify? - user_profile = get_user_profile_by_id(data['user_id']) - logging.info( - "Processing signup for user %s in realm %s", - user_profile.id, user_profile.realm.string_id, - ) - if settings.MAILCHIMP_API_KEY and settings.PRODUCTION: - endpoint = "https://{}.api.mailchimp.com/3.0/lists/{}/members".format( - settings.MAILCHIMP_API_KEY.split('-')[1], settings.ZULIP_FRIENDS_LIST_ID, - ) - params = dict(data) - del params['user_id'] - params['list_id'] = settings.ZULIP_FRIENDS_LIST_ID - params['status'] = 'subscribed' - r = requests.post(endpoint, auth=('apikey', settings.MAILCHIMP_API_KEY), json=params, timeout=10) - if r.status_code == 400 and orjson.loads(r.content)['title'] == 'Member Exists': - logging.warning("Attempted to sign up already existing email to list: %s", - data['email_address']) - elif r.status_code == 400: - retry_event(self.queue_name, data, lambda e: r.raise_for_status()) - else: - r.raise_for_status() - @assign_queue('invites') class ConfirmationEmailWorker(QueueProcessingWorker): def consume(self, data: Mapping[str, Any]) -> None: diff --git a/zproject/computed_settings.py b/zproject/computed_settings.py index f1ca16117a..1ca0292ca0 100644 --- a/zproject/computed_settings.py +++ b/zproject/computed_settings.py @@ -448,8 +448,6 @@ DROPBOX_APP_KEY = get_secret("dropbox_app_key") BIG_BLUE_BUTTON_SECRET = get_secret('big_blue_button_secret') -MAILCHIMP_API_KEY = get_secret("mailchimp_api_key") - # Twitter API credentials # Secrecy not required because its only used for R/O requests. # Please don't make us go over our rate limit.