18 flask 1x upgrade

CHANGELOG.md

@@ -2,7 +2,33 @@
 
 Records breaking changes from major version bumps
 
-## 43.0.0
+## 44.0.0
+
+PR [#455](https://github.com/alphagov/digitalmarketplace-utils/pull/455)
+
+Upgrade flask to from 0.12.4 to 1.0.2. This has breaking changes for flask apps and therefore has breaking changes for users relying on init_app.
+
+Apps should upgrade to `Flask==1.0.2` using the changelog here http://flask.pocoo.org/docs/1.0/changelog/#version-1-0-2 taking care to note
+the breaking changes in [v1.0](http://flask.pocoo.org/docs/1.0/changelog/#version-1-0) 
+
+
+Updates to DMNotifyClient and addition of `DMMandrillClient`:
+
+`DMNotifyClient.__init__ `parameter logger is now keyword-only
+
+`DMNotifyClient.get_error_message` method has been deleted
+
+`DMNotifyClient.send_email` parameter email_address has been renamed to to_email_address
+
+`DMNotifyClient.get_reference` parameter email_adress has been renamed to to_email_address
+
+dm_mandrill now contains a single class `DMMandrillClient`
+
+`dm_mandrill.send_email` has been deleted. Its functionality has been moved to `DMMandrillClient.send_email`, however the function signature has changed.
+
+`dm_mandrill.get_sent_emails` has been deleted. Its functionality has been moved to `DMMandrillClient.get_sent_emails`, however the function signature has changed.
+
+## 43.0.0
 
 PR [#447](https://github.com/alphagov/digitalmarketplace-utils/pull/447)
 
@@ -19,7 +45,7 @@ initialization of FeatureFlags for the app.
 The dependency on Flask has been upgraded to Flask 0.12, so potentially apps are going to have to make changes
 in concordance with http://flask.pocoo.org/docs/0.12/changelog/
 
-## 42.0.0
+## 42.0.0
 
 PR [#400](https://github.com/alphagov/digitalmarketplace-utils/pull/400)
 

dmutils/__init__.py

@@ -2,4 +2,4 @@
 from .flask_init import init_app, init_manager
 
 
-__version__ = '43.4.0'
+__version__ = '44.0.0'

dmutils/email/dm_mandrill.py

@@ -5,57 +5,72 @@
 
 from mandrill import Mandrill
 from dmutils.email.exceptions import EmailError
-from dmutils.email.helpers import hash_string
 from dmutils.timing import logged_duration_for_external_request as log_external_request
 
 
-def send_email(to_email_addresses, email_body, api_key, subject, from_email, from_name, tags, reply_to=None,
-               metadata=None, logger=None):
-    logger = logger or current_app.logger
-
-    if isinstance(to_email_addresses, string_types):
-        to_email_addresses = [to_email_addresses]
-
-    try:
-        mandrill_client = Mandrill(api_key)
-
-        message = {
-            'html': email_body,
-            'subject': subject,
-            'from_email': from_email,
-            'from_name': from_name,
-            'to': [{
-                'email': email_address,
-                'type': 'to'
-            } for email_address in to_email_addresses],
-            'important': False,
-            'track_opens': False,
-            'track_clicks': False,
-            'auto_text': True,
-            'tags': tags,
-            'metadata': metadata,
-            'headers': {'Reply-To': reply_to or from_email},
-            'preserve_recipients': False,
-            'recipient_metadata': [{
-                'rcpt': email_address
-            } for email_address in to_email_addresses]
-        }
-
-        with log_external_request(service='Mandrill', logger=logger):
-            result = mandrill_client.messages.send(message=message, async=True)
-
-    except Exception as e:
-        # Anything that Mandrill throws will be rethrown in a manner consistent with out other email backends.
-        # Note that this isn't just `mandrill.Error` exceptions, because the mandrill client also sometimes throws
-        # things like JSONDecodeError (sad face).
-        logger.error("Failed to send an email: {error}", extra={'error': e})
-        raise EmailError(e)
-
-    logger.info("Sent {tags} response: id={id}, email={email_hash}",
-                extra={'tags': tags, 'id': result[0]['_id'], 'email_hash': hash_string(result[0]['email'])})
-
-
-def get_sent_emails(mandrill_api_key, tags, date_from=None):
-    mandrill_client = Mandrill(mandrill_api_key)
-
-    return mandrill_client.messages.search(tags=tags, date_from=date_from, limit=1000)
+class DMMandrillClient:
+    def __init__(self, api_key=None, *, logger=None):
+        if api_key is None:
+            api_key = current_app.config["DM_MANDRILL_API_KEY"]
+
+        self.logger = logger or current_app.logger
+        self.client = Mandrill(api_key)
+
+    def get_sent_emails(self, tags, date_from=None):
+        return self.client.messages.search(tags=tags, date_from=date_from, limit=1000)
+
+    def send_email(
+        self,
+        to_email_addresses,
+        from_email_address,
+        from_name,
+        email_body,
+        subject,
+        tags,
+        reply_to=None,
+        metadata=None,
+    ):
+        if isinstance(to_email_addresses, string_types):
+            to_email_addresses = [to_email_addresses]
+
+        try:
+            message = {
+                'html': email_body,
+                'subject': subject,
+                'from_email': from_email_address,
+                'from_name': from_name,
+                'to': [{
+                    'email': email_address,
+                    'type': 'to'
+                } for email_address in to_email_addresses],
+                'important': False,
+                'track_opens': False,
+                'track_clicks': False,
+                'auto_text': True,
+                'tags': tags,
+                'metadata': metadata,
+                'headers': {'Reply-To': reply_to or from_email_address},
+                'preserve_recipients': False,
+                'recipient_metadata': [{
+                    'rcpt': email_address
+                } for email_address in to_email_addresses]
+            }
+
+            with log_external_request(service='Mandrill', logger=self.logger):
+                result = self.client.messages.send(message=message, async=True)
+
+        except Exception as e:
+            # Anything that Mandrill throws will be rethrown in a manner consistent with out other email backends.
+            # Note that this isn't just `mandrill.Error` exceptions, because the mandrill client also sometimes throws
+            # things like JSONDecodeError (sad face).
+            self.logger.error(
+                "Error sending email: {error}",
+                extra={
+                    "client": self.client.__class__,
+                    "error": e,
+                    "tags": tags,
+                },
+            )
+            raise EmailError(e)
+
+        return result

dmutils/email/dm_notify.py

@@ -17,21 +17,35 @@ class DMNotifyClient:
 
     def __init__(
             self,
-            govuk_notify_api_key,
+            govuk_notify_api_key=None,
             govuk_notify_base_url='https://api.notifications.service.gov.uk',
-            logger=None,
             redirect_domains_to_address=None,
+            *,
+            logger=None,
+            templates=None,
     ):
         """
-            :param govuk_notify_api_key:
+            :param govuk_notify_api_key: defaults to current_app.config["DM_NOTIFY_API_KEY"]
             :param govuk_notify_base_url:
-            :param logger: logger to log progress to, taken from current_app if Falsey
             :param redirect_domains_to_address: dictionary mapping email domain to redirected email address - emails
                 sent to a email with a domain in this mapping will instead be sent to the corresponding value set here.
                 if `redirect_domains_to_address` is `None` will fall back to looking for a
                 `DM_NOTIFY_REDIRECT_DOMAINS_TO_ADDRESS` setting in the current flask app's config (if available).
+
+        The following arguments are keyword-only and should only be used if you want to operate outside
+        of a Flask app context.
+            :param logger: logger to log progress to, taken from current_app if Falsey
+            :param templates: a dictionary of template names to template uuids, so that you can use
+                descriptive names when specifying a template. This defaults to current_app.config["NOTIFY_TEMPLATES"].
         """
+        if govuk_notify_api_key is None:
+            govuk_notify_api_key = current_app.config["DM_NOTIFY_API_KEY"]
+
+        if templates is None:
+            self.templates = current_app.config.get("NOTIFY_TEMPLATES", {}) if current_app else {}
+
         self.logger = logger or current_app.logger
+
         self.client = self._client_class(govuk_notify_api_key, govuk_notify_base_url)
         self._redirect_domains_to_address = (
             current_app.config.get("DM_NOTIFY_REDIRECT_DOMAINS_TO_ADDRESS")
@@ -66,54 +80,66 @@ def has_been_sent(self, reference):
         return reference in self.get_delivered_references()
 
     @staticmethod
-    def get_reference(email_address, template_id, personalisation=None):
+    def get_reference(to_email_address, template_id, personalisation=None):
         """
         Method to return the standard reference given the variables the email is sent with.
 
-        :param email_address: Emails recipient
+        :param to_email_address: Emails recipient
         :param template_id: Emails template ID on Notify
         :param personalisation: Template parameters
         :return: Hashed string 'reference' to be passed to client.send_email_notification or self.send_email
         """
         personalisation_string = u','.join(
             list(map(lambda x: u'{}'.format(x), personalisation.values()))
         ) if personalisation else u''
-        details_string = u'|'.join([email_address, template_id, personalisation_string])
+        details_string = u'|'.join([to_email_address, template_id, personalisation_string])
         return hash_string(details_string)
 
-    @staticmethod
-    def get_error_message(email_address, error):
-        """Format a logical error message from the error response."""
-        messages = []
-        message_prefix = u'Error sending message to {email_address}: '.format(email_address=email_address)
-        message_string = u'{status_code} {error}: {message}'
-
-        for message in error.message:
-            format_kwargs = {
-                'status_code': error.status_code,
-                'error': message['error'],
-                'message': message['message'],
-            }
-            messages.append(message_string.format(**format_kwargs))
-        return message_prefix + u', '.join(messages)
-
-    def send_email(self, email_address, template_id, personalisation=None, allow_resend=True, reference=None):
+    def _log_email_error_message(self, to_email_address, template_name, reference, error):
+        """Format a logical error message from the error response and send it to the logger"""
+
+        error_messages = []
+        for error_message in error.message:
+            error_messages.append(
+                "{status_code} {error}: {message}".format(
+                    status_code=error.status_code,
+                    error=error_message["error"],
+                    message=error_message["message"],
+                )
+            )
+
+        self.logger.error(
+            "Error sending email: {error_messages}",
+            extra={
+                "client": self.__class__,
+                "reference": reference,
+                "template_name": template_name,
+                "to_email_address": hash_string(to_email_address),
+                "error_messages": error_messages,
+            },
+        )
+
+    def send_email(self, to_email_address, template_name, personalisation=None, allow_resend=True, reference=None):
         """
         Method to send an email using the Notify api.
 
-        :param email_address: String email address for recipient
-        :param template_id: Template accessible on the Notify account whose  api_key you instantiated the class with
+        :param to_email_address: String email address for recipient
+        :param template_name: Template accessible on the Notify account whose api_key you instantiated the class with.
+                              Can either be a UUID or a key to the current_app.config["NOTIFY_TEMPLATES"] dictionary.
         :param personalisation: The template variables, dict
         :param allow_resend: if False instantiate the delivered reference cache and ensure we are not sending duplicates
         :return: response from the api. For more information see https://github.com/alphagov/notifications-python-client
         """
-        reference = reference or self.get_reference(email_address, template_id, personalisation)
+        template_id = self.templates.get(template_name, template_name)
+        reference = reference or self.get_reference(to_email_address, template_id, personalisation)
+
         if not allow_resend and self.has_been_sent(reference):
             self.logger.info(
-                "Email {reference} (template {template_id}) has already been sent to {email_address} through Notify",
+                "Email with reference '{reference}' has already been sent",
                 extra=dict(
-                    email_address=hash_string(email_address),
-                    template_id=template_id,
+                    client=self.client.__class__,
+                    to_email_address=hash_string(to_email_address),
+                    template_name=template_name,
                     reference=reference,
                 ),
             )
@@ -125,9 +151,9 @@ def send_email(self, email_address, template_id, personalisation=None, allow_res
             self._redirect_domains_to_address
             and self._redirect_domains_to_address.get(
                 # splitting at rightmost @ should reliably give us the domain
-                email_address.rsplit("@", 1)[-1].lower()
+                to_email_address.rsplit("@", 1)[-1].lower()
             )
-        ) or email_address
+        ) or to_email_address
 
         try:
             with log_external_request(service='Notify'):
@@ -139,7 +165,7 @@ def send_email(self, email_address, template_id, personalisation=None, allow_res
                 )
 
         except HTTPError as e:
-            self.logger.error(self.get_error_message(hash_string(email_address), e))
+            self._log_email_error_message(to_email_address, template_name, reference, e)
             raise EmailError(str(e))
 
         self._update_cache(reference)

dmutils/logging.py

@@ -101,10 +101,9 @@ def after_request(response):
     if app.config['DM_APP_NAME'] != 'search-api':
         loggers.append(logging.getLogger('urllib3.util.retry'))
 
-    for logger in loggers:
-        logger.addHandler(handler)
-        logger.setLevel(loglevel)
-
+    for logger_ in loggers:
+        logger_.addHandler(handler)
+        logger_.setLevel(loglevel)
     app.logger.info('Logging configured')
 
 

setup.py

@@ -25,7 +25,7 @@
     install_requires=[
          'Flask-Script==2.0.6',
          'Flask-WTF==0.14.2',
-         'Flask==0.12.4',
+         'Flask==1.0.2',
          'Flask-Login>=0.2.11',
          'boto3==1.7.83',
          'botocore<1.11.0',