Compare commits

..

13 Commits

Author SHA1 Message Date
David McDonald
226815b7d8 more wip 2020-06-16 13:59:15 +01:00
Pea Tyczynska
f718b71dba Make freeze requirements 2020-06-16 12:31:39 +01:00
David McDonald
e674a3ca22 wip 2020-06-16 12:22:24 +01:00
Leo Hemsted
eec2c2859e Merge pull request #2876 from alphagov/more-histograms
add more prometheus metrics
2020-06-15 17:28:59 +01:00
Leo Hemsted
58ab99d74b add more prometheus metrics
Two new metrics:

auth_db_connection_duration_seconds (histogram)
  wraps the first DB call of post notifications. This includes waiting
  to get a connection from the pool, and also making the actual request
  to the db to retrieve the service and api keys. (i'm not sure there's
  an easy way to separate these two things)

post_notification_json_parse_duration_seconds
  wraps parsing the v2 post notifications json parsing and schema
  validation. Shouldn't include any async code
2020-06-15 16:26:56 +01:00
David McDonald
4c230a7235 Merge pull request #2819 from alphagov/additional-prometheus-metrics
Additional prometheus metrics
2020-06-12 17:02:12 +01:00
David McDonald
8b4a424df1 Tidy up 2020-06-12 16:51:44 +01:00
Leo Hemsted
15ce9fe3f9 add metrics for redis timings 2020-06-12 14:52:22 +01:00
Leo Hemsted
d9b3b31a6a add loadtesting to manifest so we can deploy a separate app 2020-06-12 14:52:22 +01:00
Leo Hemsted
cd9b80f415 set test_errors app fixture to session scope
we have one global metrics variable `metrics = GDSMetrics()`, and we
then call `metrics.init_app` from within the flask application set up.
The v2/test_errors.py app_for_test fixture calls create_app, would call
metrics.init_app multiple times for the same metrics instance. This
causes errors, so change the fixture to session level so it only calls
once per test run.
2020-06-12 14:52:22 +01:00
Leo Hemsted
faa8faa0c4 bump prometheus-client to 0.7.1
there's multiple performance improvements from prometheus-client 0.2.0.
pin this bump while we wait for gds metrics client to increase its
dependency
2020-06-12 14:52:22 +01:00
Leo Hemsted
c4dc0f64c5 dd sqlalchemy connection metrics for celery tasks
grab the worker app name and task name rather than the web host and
endpoint. also add a fallback for if we're not in a web request or a
celery task. I think that'll probably happen when we use alembic, or if
we do things from within flask shell
2020-06-12 14:52:22 +01:00
Leo Hemsted
6e32ca5996 add prometheus metrics for connection pools (both web and sql)
add the following prometheus metrics to keep track of general app
instance health.

 # sqs_apply_async_duration

how long does the actual SQS call (a standard web request to AWS) take.
a histogram with default bucket sizes, split up per task that was
created.

 # concurrent_web_request_count

how many web requests is this app currently serving. this is split up
per process, so we'd expect multiple responses per app instance

 # db_connection_total_connected

how many connections does this app (process) have open to the database.
They might be idle.

 # db_connection_total_checked_out

how many connections does this app (process) have open that are
currently in use by a web worker

 # db_connection_open_duration_seconds

a histogram per endpoint of how long the db connection was taken from
the pool for. won't have any data if a connection was never opened.
2020-06-12 14:52:22 +01:00
19 changed files with 228 additions and 178 deletions

View File

@@ -1,20 +1,23 @@
import time
import os
import random
import string
import uuid
from flask import _request_ctx_stack, request, g, jsonify, make_response
from celery import current_task
from flask import _request_ctx_stack, request, g, jsonify, make_response, current_app, has_request_context
from flask_sqlalchemy import SQLAlchemy as _SQLAlchemy
from flask_marshmallow import Marshmallow
from flask_migrate import Migrate
from gds_metrics import GDSMetrics
from gds_metrics.metrics import Gauge, Histogram
from time import monotonic
from notifications_utils.clients.zendesk.zendesk_client import ZendeskClient
from notifications_utils.clients.statsd.statsd_client import StatsdClient
from notifications_utils.clients.redis import RequestCache
from notifications_utils.clients.redis.redis_client import RedisClient
from notifications_utils.clients.encryption.encryption_client import Encryption
from notifications_utils import logging, request_helper
from sqlalchemy import event
from werkzeug.exceptions import HTTPException as WerkzeugHTTPException
from werkzeug.local import LocalProxy
@@ -56,7 +59,6 @@ encryption = Encryption()
zendesk_client = ZendeskClient()
statsd_client = StatsdClient()
redis_store = RedisClient()
request_cache = RequestCache(redis_store)
performance_platform_client = PerformancePlatformClient()
document_download_client = DocumentDownloadClient()
metrics = GDSMetrics()
@@ -66,6 +68,11 @@ clients = Clients()
api_user = LocalProxy(lambda: _request_ctx_stack.top.api_user)
authenticated_service = LocalProxy(lambda: _request_ctx_stack.top.authenticated_service)
CONCURRENT_REQUESTS = Gauge(
'concurrent_web_request_count',
'How many concurrent requests are currently being served',
)
def create_app(application):
from app.config import configs
@@ -110,6 +117,9 @@ def create_app(application):
from app.commands import setup_commands
setup_commands(application)
# set up sqlalchemy events
setup_sqlalchemy_events(application)
return application
@@ -257,17 +267,22 @@ def register_v2_blueprints(application):
def init_app(app):
@app.before_request
def record_user_agent():
statsd_client.incr("user-agent.{}".format(process_user_agent(request.headers.get('User-Agent', None))))
@app.before_request
def record_request_details():
CONCURRENT_REQUESTS.inc()
g.start = monotonic()
g.endpoint = request.endpoint
@app.after_request
def after_request(response):
CONCURRENT_REQUESTS.dec()
response.headers.add('Access-Control-Allow-Origin', '*')
response.headers.add('Access-Control-Allow-Headers', 'Content-Type,Authorization')
response.headers.add('Access-Control-Allow-Methods', 'GET,PUT,POST,DELETE')
@@ -313,3 +328,83 @@ def process_user_agent(user_agent_string):
return "non-notify-user-agent"
else:
return "unknown"
def setup_sqlalchemy_events(app):
TOTAL_DB_CONNECTIONS = Gauge(
'db_connection_total_connected',
'How many db connections are currently held (potentially idle) by the server',
)
TOTAL_CHECKED_OUT_DB_CONNECTIONS = Gauge(
'db_connection_total_checked_out',
'How many db connections are currently checked out by web requests',
)
DB_CONNECTION_OPEN_DURATION_SECONDS = Histogram(
'db_connection_open_duration_seconds',
'How long db connections are held open for in seconds',
['method', 'host', 'path']
)
# need this or db.engine isn't accessible
with app.app_context():
@event.listens_for(db.engine, 'connect')
def connect(dbapi_connection, connection_record):
# connection first opened with db
TOTAL_DB_CONNECTIONS.inc()
@event.listens_for(db.engine, 'close')
def close(dbapi_connection, connection_record):
# connection closed (probably only happens with overflow connections)
TOTAL_DB_CONNECTIONS.dec()
@event.listens_for(db.engine, 'checkout')
def checkout(dbapi_connection, connection_record, connection_proxy):
# connection given to a web worker
TOTAL_CHECKED_OUT_DB_CONNECTIONS.inc()
# this will overwrite any previous checkout_at timestamp
connection_record.info['checkout_at'] = time.monotonic()
# checkin runs after the request is already torn down, therefore we add the request_data onto the
# connection_record as otherwise it won't have that information when checkin actually runs.
# Note: this is not a problem for checkouts as the checkout always happens within a web request or task
# web requests
if has_request_context():
connection_record.info['request_data'] = {
'method': request.method,
'host': request.host,
'url_rule': request.url_rule.rule if request.url_rule else 'No endpoint'
}
# celery apps
elif current_task:
connection_record.info['request_data'] = {
'method': 'celery',
'host': current_app.config['NOTIFY_APP_NAME'], # worker name
'url_rule': current_task.name, # task name
}
# anything else. migrations possibly.
else:
current_app.logger.warning('Checked out sqlalchemy connection from outside of request/task')
connection_record.info['request_data'] = {
'method': 'unknown',
'host': 'unknown',
'url_rule': 'unknown',
}
@event.listens_for(db.engine, 'checkin')
def checkin(dbapi_connection, connection_record):
# connection returned by a web worker
TOTAL_CHECKED_OUT_DB_CONNECTIONS.dec()
# duration that connection was held by a single web request
duration = time.monotonic() - connection_record.info['checkout_at']
DB_CONNECTION_OPEN_DURATION_SECONDS.labels(
connection_record.info['request_data']['method'],
connection_record.info['request_data']['host'],
connection_record.info['request_data']['url_rule']
).observe(duration)

View File

@@ -6,12 +6,18 @@ from notifications_python_client.errors import (
from notifications_utils import request_helper
from sqlalchemy.exc import DataError
from sqlalchemy.orm.exc import NoResultFound
from gds_metrics import Histogram
from app.dao.services_dao import dao_fetch_service_by_id_with_api_keys
GENERAL_TOKEN_ERROR_MESSAGE = 'Invalid token: make sure your API token matches the example at https://docs.notifications.service.gov.uk/rest-api.html#authorisation-header' # noqa
AUTH_DB_CONNECTION_DURATION_SECONDS = Histogram(
'auth_db_connection_duration_seconds',
'Time taken to get DB connection and fetch service from database',
)
class AuthError(Exception):
def __init__(self, message, code, service_id=None, api_key_id=None):
@@ -87,7 +93,8 @@ def requires_auth():
issuer = __get_token_issuer(auth_token) # ie the `iss` claim which should be a service ID
try:
service = dao_fetch_service_by_id_with_api_keys(issuer)
with AUTH_DB_CONNECTION_DURATION_SECONDS.time():
service = dao_fetch_service_by_id_with_api_keys(issuer)
except DataError:
raise AuthError("Invalid token: service id is not the right data type", 403)
except NoResultFound:

View File

@@ -1,5 +1,6 @@
import time
from gds_metrics.metrics import Histogram
from celery import Celery, Task
from celery.signals import worker_process_shutdown
from flask import g, request
@@ -19,6 +20,12 @@ def log_on_worker_shutdown(sender, signal, pid, exitcode, **kwargs):
def make_task(app):
SQS_APPLY_ASYNC_DURATION_SECONDS = Histogram(
'sqs_apply_async_duration_seconds',
'Time taken to put task on queue',
['task_name']
)
class NotifyTask(Task):
abstract = True
start = None
@@ -52,7 +59,8 @@ def make_task(app):
if has_request_context() and hasattr(request, 'request_id'):
kwargs['request_id'] = request.request_id
return super().apply_async(args, kwargs, task_id, producer, link, link_error, **options)
with SQS_APPLY_ASYNC_DURATION_SECONDS.labels(self.name).time():
return super().apply_async(args, kwargs, task_id, producer, link, link_error, **options)
return NotifyTask

View File

@@ -5,22 +5,14 @@ from app.models import LETTER_TYPE
from app.notifications.process_notifications import persist_notification
def create_letter_notification(
letter_data,
template,
service,
api_key,
status,
reply_to_text=None,
billable_units=None,
):
def create_letter_notification(letter_data, template, api_key, status, reply_to_text=None, billable_units=None):
notification = persist_notification(
template_id=template.id,
template_version=template._template['version'],
template_postage=template._template['postage'],
template_version=template.version,
template_postage=template.postage,
# we only accept addresses_with_underscores from the API (from CSV we also accept dashes, spaces etc)
recipient=PostalAddress.from_personalisation(letter_data['personalisation']).normalised,
service=service,
service=template.service,
personalisation=letter_data['personalisation'],
notification_type=LETTER_TYPE,
api_key_id=api_key.id,

View File

@@ -9,11 +9,6 @@ from notifications_utils.recipients import (
validate_and_format_phone_number,
format_email_address
)
from notifications_utils.template import (
PlainTextEmailTemplate,
SMSMessageTemplate,
LetterPrintTemplate,
)
from notifications_utils.timezones import convert_bst_to_utc
from app import redis_store
@@ -39,18 +34,17 @@ from app.dao.notifications_dao import (
from app.v2.errors import BadRequestError
def create_content_for_notification(template_dict, personalisation):
if template_dict['template_type'] == EMAIL_TYPE:
template_object = PlainTextEmailTemplate(template_dict, personalisation)
if template_dict['template_type'] == SMS_TYPE:
template_object = SMSMessageTemplate(template_dict, personalisation)
if template_dict['template_type'] == LETTER_TYPE:
template_object = LetterPrintTemplate(
template_dict,
personalisation,
contact_block=template_dict['reply_to_text'],
)
from gds_metrics import Histogram
REDIS_GET_AND_INCR_DAILY_LIMIT_DURATION_SECONDS = Histogram(
'redis_get_and_incr_daily_limit_duration_seconds',
'Time taken to get and possibly incremement the daily limit cache key',
)
def create_content_for_notification(template, personalisation):
template_object = template._as_utils_template_with_personalisation(personalisation)
check_placeholders(template_object)
return template_object
@@ -130,8 +124,9 @@ def persist_notification(
if not simulated:
dao_create_notification(notification)
if key_type != KEY_TYPE_TEST:
if redis_store.get(redis.daily_limit_cache_key(service.id)):
redis_store.incr(redis.daily_limit_cache_key(service.id))
with REDIS_GET_AND_INCR_DAILY_LIMIT_DURATION_SECONDS.time():
if redis_store.get(redis.daily_limit_cache_key(service.id)):
redis_store.incr(redis.daily_limit_cache_key(service.id))
current_app.logger.info(
"{} {} created at {}".format(notification_type, notification_id, notification_created_at)

View File

@@ -100,13 +100,12 @@ def send_notification(notification_type):
check_rate_limiting(authenticated_service, api_user)
template_with_content = validate_template(
template, template_with_content = validate_template(
template_id=notification_form['template'],
personalisation=notification_form.get('personalisation', {}),
service=authenticated_service,
notification_type=notification_type
)
template_dict = template_with_content._template
_service_allowed_to_send_to(notification_form, authenticated_service)
if not service_has_permission(notification_type, authenticated_service.permissions):
@@ -119,9 +118,9 @@ def send_notification(notification_type):
_service_can_send_internationally(authenticated_service, notification_form['to'])
# Do not persist or send notification to the queue if it is a simulated recipient
simulated = simulated_recipient(notification_form['to'], notification_type)
notification_model = persist_notification(template_id=template_dict['id'],
template_version=template_dict['version'],
template_postage=template_dict['postage'],
notification_model = persist_notification(template_id=template.id,
template_version=template.version,
template_postage=template.postage,
recipient=request.get_json()['to'],
service=authenticated_service,
personalisation=notification_form.get('personalisation', None),
@@ -129,16 +128,16 @@ def send_notification(notification_type):
api_key_id=api_user.id,
key_type=api_user.key_type,
simulated=simulated,
reply_to_text=template_dict['reply_to_text']
reply_to_text=template.get_reply_to_text()
)
if not simulated:
queue_name = QueueNames.PRIORITY if template_dict['process_type'] == PRIORITY else None
queue_name = QueueNames.PRIORITY if template.process_type == PRIORITY else None
send_notification_to_queue(notification=notification_model,
research_mode=authenticated_service.research_mode,
queue=queue_name)
else:
current_app.logger.debug("POST simulated notification for id: {}".format(notification_model.id))
notification_form.update({"template_version": template_dict['version']})
notification_form.update({"template_version": template.version})
return jsonify(
data=get_notification_return_data(

View File

@@ -16,21 +16,30 @@ from app.models import (
)
from app.service.utils import service_allowed_to_send_to
from app.v2.errors import TooManyRequestsError, BadRequestError, RateLimitError
from app import redis_store, request_cache
from app import redis_store
from app.notifications.process_notifications import create_content_for_notification
from app.utils import get_public_notify_type_text
from app.dao.service_email_reply_to_dao import dao_get_reply_to_by_id
from app.dao.service_letter_contact_dao import dao_get_letter_contact_by_id
from gds_metrics.metrics import Histogram
REDIS_EXCEEDED_RATE_LIMIT_DURATION_SECONDS = Histogram(
'redis_exceeded_rate_limit_duration_seconds',
'Time taken to check rate limit',
)
def check_service_over_api_rate_limit(service, api_key):
if current_app.config['API_RATE_LIMIT_ENABLED'] and current_app.config['REDIS_ENABLED']:
cache_key = rate_limit_cache_key(service.id, api_key.key_type)
rate_limit = service.rate_limit
interval = 60
if redis_store.exceeded_rate_limit(cache_key, rate_limit, interval):
current_app.logger.info("service {} has been rate limited for throughput".format(service.id))
raise RateLimitError(rate_limit, interval, api_key.key_type)
with REDIS_EXCEEDED_RATE_LIMIT_DURATION_SECONDS.time():
if redis_store.exceeded_rate_limit(cache_key, rate_limit, interval):
current_app.logger.info("service {} has been rate limited for throughput".format(service.id))
raise RateLimitError(rate_limit, interval, api_key.key_type)
def check_service_over_daily_message_limit(key_type, service):
@@ -62,7 +71,7 @@ def check_template_is_for_notification_type(notification_type, template_type):
def check_template_is_active(template):
if template['archived']:
if template.archived:
raise BadRequestError(fields=[{'template': 'Template has been deleted'}],
message="Template has been deleted")
@@ -138,26 +147,18 @@ def check_notification_content_is_not_empty(template_with_content):
raise BadRequestError(message=message)
@request_cache.set('template-{template_id}-version-None')
def get_template_dict(template_id, service_id):
from app.schemas import template_schema
def validate_template(template_id, personalisation, service, notification_type):
try:
fetched_template = templates_dao.dao_get_template_by_id_and_service_id(
template = templates_dao.dao_get_template_by_id_and_service_id(
template_id=template_id,
service_id=service_id
service_id=service.id
)
except NoResultFound:
message = 'Template not found'
raise BadRequestError(message=message,
fields=[{'template': message}])
return template_schema.dump(fetched_template).data
def validate_template(template_id, personalisation, service, notification_type):
template = get_template_dict(template_id, service.id)
check_template_is_for_notification_type(notification_type, template['template_type'])
check_template_is_for_notification_type(notification_type, template.template_type)
check_template_is_active(template)
template_with_content = create_content_for_notification(template, personalisation)
@@ -166,7 +167,7 @@ def validate_template(template_id, personalisation, service, notification_type):
check_content_char_count(template_with_content)
return template_with_content
return template, template_with_content
def check_reply_to(service_id, reply_to_id, type_):

View File

@@ -2,7 +2,6 @@ from datetime import (
datetime,
date,
timedelta)
from uuid import UUID
from flask_marshmallow.fields import fields
from marshmallow import (
post_load,
@@ -335,16 +334,6 @@ class TemplateSchema(BaseTemplateSchema):
if not subject or subject.strip() == '':
raise ValidationError('Invalid template subject', 'subject')
@post_dump()
def __post_dump(self, data):
for field in (
'service',
'created_by',
'template_redacted',
):
if isinstance(data[field], UUID):
data[field] = str(data[field])
class TemplateHistorySchema(BaseSchema):

View File

@@ -7,6 +7,7 @@ from boto.exception import SQSError
from flask import request, jsonify, current_app, abort
from notifications_utils.postal_address import PostalAddress
from notifications_utils.recipients import try_validate_and_format_phone_number
from gds_metrics import Histogram
from app import (
api_user,
@@ -42,7 +43,6 @@ from app.notifications.process_letter_notifications import (
create_letter_notification
)
from app.notifications.process_notifications import (
create_content_for_notification,
persist_notification,
persist_scheduled_notification,
send_notification_to_queue,
@@ -75,9 +75,14 @@ from app.v2.notifications.notification_schemas import (
from app.v2.utils import get_valid_json
POST_NOTIFICATION_JSON_PARSE_DURATION_SECONDS = Histogram(
'post_notification_json_parse_duration_seconds',
'Time taken to parse and validate post request json',
)
@v2_notification_blueprint.route('/{}'.format(LETTER_TYPE), methods=['POST'])
def post_precompiled_letter_notification():
from app.schemas import template_schema
request_json = get_valid_json()
if 'content' not in (request_json or {}):
return post_notification(LETTER_TYPE)
@@ -90,9 +95,6 @@ def post_precompiled_letter_notification():
check_rate_limiting(authenticated_service, api_user)
template = get_precompiled_letter_template(authenticated_service.id)
template = create_content_for_notification(
template_schema.dump(template).data, {}
)
# For precompiled letters the to field will be set to Provided as PDF until the validation passes,
# then the address of the letter will be set as the to field
@@ -100,12 +102,13 @@ def post_precompiled_letter_notification():
'address_line_1': 'Provided as PDF'
}
reply_to = get_reply_to_text(LETTER_TYPE, form, template)
notification = process_letter_notification(
letter_data=form,
api_key=api_user,
template=template,
service=authenticated_service,
reply_to_text=template._template['reply_to_text'],
reply_to_text=reply_to,
precompiled=True
)
@@ -120,16 +123,17 @@ def post_precompiled_letter_notification():
@v2_notification_blueprint.route('/<notification_type>', methods=['POST'])
def post_notification(notification_type):
request_json = get_valid_json()
with POST_NOTIFICATION_JSON_PARSE_DURATION_SECONDS.time():
request_json = get_valid_json()
if notification_type == EMAIL_TYPE:
form = validate(request_json, post_email_request)
elif notification_type == SMS_TYPE:
form = validate(request_json, post_sms_request)
elif notification_type == LETTER_TYPE:
form = validate(request_json, post_letter_request)
else:
abort(404)
if notification_type == EMAIL_TYPE:
form = validate(request_json, post_email_request)
elif notification_type == SMS_TYPE:
form = validate(request_json, post_sms_request)
elif notification_type == LETTER_TYPE:
form = validate(request_json, post_letter_request)
else:
abort(404)
check_service_has_permission(notification_type, authenticated_service.permissions)
@@ -139,21 +143,20 @@ def post_notification(notification_type):
check_rate_limiting(authenticated_service, api_user)
template_with_content = validate_template(
template, template_with_content = validate_template(
form['template_id'],
form.get('personalisation', {}),
authenticated_service,
notification_type,
)
reply_to = get_reply_to_text(notification_type, form, template_with_content)
reply_to = get_reply_to_text(notification_type, form, template)
if notification_type == LETTER_TYPE:
notification = process_letter_notification(
letter_data=form,
api_key=api_user,
template=template_with_content,
service=authenticated_service,
template=template,
reply_to_text=reply_to
)
else:
@@ -161,12 +164,11 @@ def post_notification(notification_type):
form=form,
notification_type=notification_type,
api_key=api_user,
template=template_with_content,
template=template,
service=authenticated_service,
reply_to_text=reply_to
)
# Think this is redundant
template_with_content.values = notification.personalisation
if notification_type == SMS_TYPE:
@@ -243,7 +245,7 @@ def process_sms_or_email_notification(*, form, notification_type, api_key, templ
notification = persist_notification(
notification_id=notification_id,
template_id=template.id,
template_version=template._template['version'],
template_version=template.version,
recipient=form_send_to,
service=service,
personalisation=personalisation,
@@ -261,7 +263,7 @@ def process_sms_or_email_notification(*, form, notification_type, api_key, templ
persist_scheduled_notification(notification.id, form["scheduled_for"])
else:
if not simulated:
queue_name = QueueNames.PRIORITY if template._template['process_type'] == PRIORITY else None
queue_name = QueueNames.PRIORITY if template.process_type == PRIORITY else None
send_notification_to_queue(
notification=notification,
research_mode=service.research_mode,
@@ -288,7 +290,7 @@ def save_email_to_queue(
data = {
"id": notification_id,
"template_id": str(template.id),
"template_version": template._template['version'],
"template_version": template.version,
"to": form['email_address'],
"service_id": str(service_id),
"personalisation": personalisation,
@@ -339,7 +341,7 @@ def process_document_uploads(personalisation_data, service, simulated=False):
return personalisation_data, len(file_keys)
def process_letter_notification(*, letter_data, api_key, template, service, reply_to_text, precompiled=False):
def process_letter_notification(*, letter_data, api_key, template, reply_to_text, precompiled=False):
if api_key.key_type == KEY_TYPE_TEAM:
raise BadRequestError(message='Cannot send letters with a team api key', status_code=403)
@@ -350,7 +352,6 @@ def process_letter_notification(*, letter_data, api_key, template, service, repl
return process_precompiled_letter_notifications(letter_data=letter_data,
api_key=api_key,
template=template,
service=service,
reply_to_text=reply_to_text)
address = PostalAddress.from_personalisation(
@@ -385,7 +386,6 @@ def process_letter_notification(*, letter_data, api_key, template, service, repl
notification = create_letter_notification(letter_data=letter_data,
template=template,
service=service,
api_key=api_key,
status=status,
reply_to_text=reply_to_text)
@@ -407,7 +407,7 @@ def process_letter_notification(*, letter_data, api_key, template, service, repl
return notification
def process_precompiled_letter_notifications(*, letter_data, api_key, template, service, reply_to_text):
def process_precompiled_letter_notifications(*, letter_data, api_key, template, reply_to_text):
try:
status = NOTIFICATION_PENDING_VIRUS_CHECK
letter_content = base64.b64decode(letter_data['content'])
@@ -416,7 +416,6 @@ def process_precompiled_letter_notifications(*, letter_data, api_key, template,
notification = create_letter_notification(letter_data=letter_data,
template=template,
service=service,
api_key=api_key,
status=status,
reply_to_text=reply_to_text)
@@ -448,7 +447,7 @@ def get_reply_to_text(notification_type, form, template):
service_email_reply_to_id = form.get("email_reply_to_id", None)
reply_to = check_service_email_reply_to_id(
str(authenticated_service.id), service_email_reply_to_id, notification_type
) or template._template['reply_to_text']
) or template.get_reply_to_text()
elif notification_type == SMS_TYPE:
service_sms_sender_id = form.get("sms_sender_id", None)
@@ -458,9 +457,9 @@ def get_reply_to_text(notification_type, form, template):
if sms_sender_id:
reply_to = try_validate_and_format_phone_number(sms_sender_id)
else:
reply_to = template._template['reply_to_text']
reply_to = template.get_reply_to_text()
elif notification_type == LETTER_TYPE:
reply_to = template._template['reply_to_text']
reply_to = template.get_reply_to_text()
return reply_to

View File

@@ -2,9 +2,12 @@
from __future__ import print_function
from flask import Flask
import psycogreen.eventlet
from app import create_app
psycogreen.eventlet.patch_psycopg()
application = Flask('app')
create_app(application)

View File

@@ -6,7 +6,7 @@ import gunicorn
from gds_metrics.gunicorn import child_exit # noqa
workers = 4
worker_class = "eventlet"
worker_class = "gevent"
worker_connections = 256
errorlog = "/home/vcap/logs/gunicorn_error.log"
bind = "0.0.0.0:{}".format(os.getenv("PORT"))

View File

@@ -1,6 +1,6 @@
{%- set app_vars = {
'notify-api': {
'NOTIFY_APP_NAME': 'api',
'notify-api-canary1': {
'NOTIFY_APP_NAME': 'api-canary1',
'disk_quota': '2G',
'sqlalchemy_pool_size': 30,
'routes': {
@@ -12,8 +12,8 @@
'health-check-invocation-timeout': 3,
'instances': {
'preview': None,
'staging': None,
'production': 25
'staging': 1,
'production': 1
},
},
'notify-api-db-migration': {

View File

@@ -10,13 +10,14 @@ Flask-Migrate==2.5.3
git+https://github.com/mitsuhiko/flask-sqlalchemy.git@500e732dd1b975a56ab06a46bd1a20a21e682262#egg=Flask-SQLAlchemy==2.3.2.dev20190108
Flask==1.1.2
click-datetime==0.2
eventlet==0.25.2
gevent==20.6.1
gunicorn==20.0.4
iso8601==0.1.12
itsdangerous==1.1.0
jsonschema==3.2.0
marshmallow-sqlalchemy==0.23.0
marshmallow==2.21.0 # pyup: <3 # v3 throws errors
psycogreen==1.0.2
psycopg2-binary==2.8.5
PyJWT==1.7.1
SQLAlchemy==1.3.17
@@ -28,4 +29,6 @@ awscli-cwlogs>=1.4,<1.5
git+https://github.com/alphagov/notifications-utils.git@39.4.4#egg=notifications-utils==39.4.4
# gds-metrics requires prometheseus 0.2.0, override that requirement as 0.7.1 brings significant performance gains
prometheus-client==0.7.1
gds-metrics==0.2.0

View File

@@ -12,13 +12,14 @@ Flask-Migrate==2.5.3
git+https://github.com/mitsuhiko/flask-sqlalchemy.git@500e732dd1b975a56ab06a46bd1a20a21e682262#egg=Flask-SQLAlchemy==2.3.2.dev20190108
Flask==1.1.2
click-datetime==0.2
eventlet==0.25.2
gevent==20.6.1
gunicorn==20.0.4
iso8601==0.1.12
itsdangerous==1.1.0
jsonschema==3.2.0
marshmallow-sqlalchemy==0.23.0
marshmallow==2.21.0 # pyup: <3 # v3 throws errors
psycogreen==1.0.2
psycopg2-binary==2.8.5
PyJWT==1.7.1
SQLAlchemy==1.3.17
@@ -30,6 +31,8 @@ awscli-cwlogs>=1.4,<1.5
git+https://github.com/alphagov/notifications-utils.git@39.4.4#egg=notifications-utils==39.4.4
# gds-metrics requires prometheseus 0.2.0, override that requirement as 0.7.1 brings significant performance gains
prometheus-client==0.7.1
gds-metrics==0.2.0
## The following requirements were added by pip freeze:
@@ -37,19 +40,18 @@ alembic==1.4.2
amqp==1.4.9
anyjson==0.3.3
attrs==19.3.0
awscli==1.18.75
awscli==1.18.80
bcrypt==3.1.7
billiard==3.3.0.23
bleach==3.1.4
blinker==1.4
boto==2.49.0
boto3==1.10.38
botocore==1.16.25
botocore==1.17.3
certifi==2020.4.5.2
chardet==3.0.4
click==7.1.2
colorama==0.4.3
dnspython==1.16.0
docutils==0.15.2
flask-redis==0.4.0
future==0.18.2
@@ -66,7 +68,6 @@ mistune==0.8.4
monotonic==1.5
orderedset==2.0.1
phonenumbers==8.11.2
prometheus-client==0.2.0
pyasn1==0.4.8
pycparser==2.20
PyPDF2==1.26.0
@@ -87,3 +88,5 @@ urllib3==1.25.9
webencodings==0.5.1
Werkzeug==1.0.1
zipp==3.1.0
zope.event==4.4
zope.interface==5.1.0

View File

@@ -1,6 +1,6 @@
#!/bin/bash
case $NOTIFY_APP_NAME in
api)
api|api-canary1)
unset GUNICORN_CMD_ARGS
exec scripts/run_app_paas.sh gunicorn -c /home/vcap/app/gunicorn_config.py application
;;

View File

@@ -2,8 +2,6 @@ from app.models import LETTER_TYPE
from app.models import Notification
from app.models import NOTIFICATION_CREATED
from app.notifications.process_letter_notifications import create_letter_notification
from app.notifications.process_notifications import create_content_for_notification
from app.notifications.validators import get_template_dict
def test_create_letter_notification_creates_notification(sample_letter_template, sample_api_key):
@@ -15,17 +13,7 @@ def test_create_letter_notification_creates_notification(sample_letter_template,
}
}
template = create_content_for_notification(get_template_dict(
sample_letter_template.id, sample_letter_template.service_id
), {})
notification = create_letter_notification(
data,
template,
sample_letter_template.service,
sample_api_key,
NOTIFICATION_CREATED,
)
notification = create_letter_notification(data, sample_letter_template, sample_api_key, NOTIFICATION_CREATED)
assert notification == Notification.query.one()
assert notification.job is None
@@ -50,17 +38,7 @@ def test_create_letter_notification_sets_reference(sample_letter_template, sampl
'reference': 'foo'
}
template = create_content_for_notification(get_template_dict(
sample_letter_template.id, sample_letter_template.service_id
), {})
notification = create_letter_notification(
data,
template,
sample_letter_template.service,
sample_api_key,
NOTIFICATION_CREATED,
)
notification = create_letter_notification(data, sample_letter_template, sample_api_key, NOTIFICATION_CREATED)
assert notification.client_reference == 'foo'
@@ -74,17 +52,7 @@ def test_create_letter_notification_sets_billable_units(sample_letter_template,
},
}
template = create_content_for_notification(get_template_dict(
sample_letter_template.id, sample_letter_template.service_id
), {})
notification = create_letter_notification(
data,
template,
sample_letter_template.service,
sample_api_key,
NOTIFICATION_CREATED,
billable_units=3,
)
notification = create_letter_notification(data, sample_letter_template, sample_api_key, NOTIFICATION_CREATED,
billable_units=3)
assert notification.billable_units == 3

View File

@@ -21,7 +21,6 @@ from app.notifications.process_notifications import (
send_notification_to_queue,
simulated_recipient
)
from app.notifications.validators import get_template_dict
from notifications_utils.recipients import validate_and_format_phone_number, validate_and_format_email_address
from app.v2.errors import BadRequestError
from tests.app.db import create_service, create_template
@@ -29,30 +28,26 @@ from tests.app.db import create_service, create_template
def test_create_content_for_notification_passes(sample_email_template):
template = Template.query.get(sample_email_template.id)
template_dict = get_template_dict(template.id, template.service_id)
content = create_content_for_notification(template_dict, None)
content = create_content_for_notification(template, None)
assert str(content) == template.content + '\n'
def test_create_content_for_notification_with_placeholders_passes(sample_template_with_placeholders):
template = Template.query.get(sample_template_with_placeholders.id)
template_dict = get_template_dict(template.id, template.service_id)
content = create_content_for_notification(template_dict, {'name': 'Bobby'})
content = create_content_for_notification(template, {'name': 'Bobby'})
assert content.content == template.content
assert 'Bobby' in str(content)
def test_create_content_for_notification_fails_with_missing_personalisation(sample_template_with_placeholders):
template = Template.query.get(sample_template_with_placeholders.id)
template_dict = get_template_dict(template.id, template.service_id)
with pytest.raises(BadRequestError):
create_content_for_notification(template_dict, None)
create_content_for_notification(template, None)
def test_create_content_for_notification_allows_additional_personalisation(sample_template_with_placeholders):
template = Template.query.get(sample_template_with_placeholders.id)
template_dict = get_template_dict(template.id, template.service_id)
create_content_for_notification(template_dict, {'name': 'Bobby', 'Additional placeholder': 'Data'})
create_content_for_notification(template, {'name': 'Bobby', 'Additional placeholder': 'Data'})
@freeze_time("2016-01-01 11:09:00.061258")

View File

@@ -19,7 +19,6 @@ from app.notifications.validators import (
check_service_sms_sender_id,
check_service_letter_contact_id,
check_reply_to,
get_template_dict,
service_can_send_to_recipient,
validate_and_format_recipient,
validate_template,
@@ -176,17 +175,15 @@ def test_check_template_is_for_notification_type_fails_when_template_type_does_n
def test_check_template_is_active_passes(sample_template):
template_dict = get_template_dict(sample_template.id, sample_template.service_id)
assert check_template_is_active(template_dict) is None
assert check_template_is_active(sample_template) is None
def test_check_template_is_active_fails(sample_template):
sample_template.archived = True
from app.dao.templates_dao import dao_update_template
dao_update_template(sample_template)
template_dict = get_template_dict(sample_template.id, sample_template.service_id)
with pytest.raises(BadRequestError) as e:
check_template_is_active(template_dict)
check_template_is_active(sample_template)
assert e.value.status_code == 400
assert e.value.message == 'Template has been deleted'
assert e.value.fields == [{'template': 'Template has been deleted'}]
@@ -317,11 +314,11 @@ def test_check_content_char_count_passes_for_long_email_or_letter(sample_service
def test_check_notification_content_is_not_empty_passes(notify_api, mocker, sample_service):
template_id = create_template(sample_service, content="Content is not empty").id
template_dict = get_template_dict(
template = templates_dao.dao_get_template_by_id_and_service_id(
template_id=template_id,
service_id=sample_service.id
)
template_with_content = create_content_for_notification(template_dict, {})
template_with_content = create_content_for_notification(template, {})
assert check_notification_content_is_not_empty(template_with_content) is None
@@ -333,11 +330,11 @@ def test_check_notification_content_is_not_empty_fails(
notify_api, mocker, sample_service, template_content, notification_values
):
template_id = create_template(sample_service, content=template_content).id
template_dict = get_template_dict(
template = templates_dao.dao_get_template_by_id_and_service_id(
template_id=template_id,
service_id=sample_service.id
)
template_with_content = create_content_for_notification(template_dict, notification_values)
template_with_content = create_content_for_notification(template, notification_values)
with pytest.raises(BadRequestError) as e:
check_notification_content_is_not_empty(template_with_content)
assert e.value.status_code == 400
@@ -352,10 +349,6 @@ def test_validate_template(sample_service):
def test_validate_template_calls_all_validators(mocker, fake_uuid, sample_service):
template = create_template(sample_service, template_type="email")
template_dict = get_template_dict(
template_id=template.id,
service_id=sample_service.id
)
mock_check_type = mocker.patch('app.notifications.validators.check_template_is_for_notification_type')
mock_check_if_active = mocker.patch('app.notifications.validators.check_template_is_active')
mock_create_conent = mocker.patch(
@@ -366,8 +359,8 @@ def test_validate_template_calls_all_validators(mocker, fake_uuid, sample_servic
validate_template(template.id, {}, sample_service, "email")
mock_check_type.assert_called_once_with("email", "email")
mock_check_if_active.assert_called_once_with(template_dict)
mock_create_conent.assert_called_once_with(template_dict, {})
mock_check_if_active.assert_called_once_with(template)
mock_create_conent.assert_called_once_with(template, {})
mock_check_not_empty.assert_called_once_with("content")
mock_check_message_is_too_long.assert_called_once_with("content")

View File

@@ -4,7 +4,7 @@ from sqlalchemy.exc import DataError
@pytest.fixture(scope='function')
def app_for_test(mocker):
def app_for_test():
import flask
from flask import Blueprint
from app.authentication.auth import AuthError