mirror of
https://github.com/GSA/notifications-api.git
synced 2026-01-30 14:31:57 -05:00
DRY-up recording the outcome of SMS sending
This reduces the code to copy when we add a new provider. I don't think we need to log the URL or status code each time: - The URL is always the same. - A "200" status code is implicit in "success". - Other status codes will be reported as exceptions. Removing these specific elements means "record_outcome" is generic and can be de-duplicated in the base class.
This commit is contained in:
@@ -18,6 +18,19 @@ class SmsClient(Client):
|
||||
Base Sms client for sending smss.
|
||||
'''
|
||||
|
||||
def record_outcome(self, success):
|
||||
log_message = "Provider request for {} {}".format(
|
||||
self.name,
|
||||
"succeeded" if success else "failed",
|
||||
)
|
||||
|
||||
if success:
|
||||
self.current_app.logger.info(log_message)
|
||||
self.statsd_client.incr(f"clients.{self.name}.success")
|
||||
else:
|
||||
self.statsd_client.incr(f"clients.{self.name}.error")
|
||||
self.current_app.logger.warning(log_message)
|
||||
|
||||
def send_sms(self, *args, **kwargs):
|
||||
raise NotImplementedError('TODO Need to implement.')
|
||||
|
||||
|
||||
@@ -75,23 +75,6 @@ class FiretextClient(SmsClient):
|
||||
def name(self):
|
||||
return 'firetext'
|
||||
|
||||
def record_outcome(self, success, response):
|
||||
status_code = response.status_code if response else 503
|
||||
|
||||
log_message = "API {} request {} on {} response status_code {}".format(
|
||||
"POST",
|
||||
"succeeded" if success else "failed",
|
||||
self.url,
|
||||
status_code
|
||||
)
|
||||
|
||||
if success:
|
||||
self.current_app.logger.info(log_message)
|
||||
self.statsd_client.incr("clients.firetext.success")
|
||||
else:
|
||||
self.statsd_client.incr("clients.firetext.error")
|
||||
self.current_app.logger.warning(log_message)
|
||||
|
||||
def send_sms(self, to, content, reference, international, sender=None):
|
||||
data = {
|
||||
"apiKey": self.international_api_key if international else self.api_key,
|
||||
@@ -115,11 +98,11 @@ class FiretextClient(SmsClient):
|
||||
if response.json()['code'] != 0:
|
||||
raise ValueError()
|
||||
except (ValueError, AttributeError) as e:
|
||||
self.record_outcome(False, response)
|
||||
self.record_outcome(False)
|
||||
raise FiretextClientResponseException(response=response, exception=e)
|
||||
self.record_outcome(True, response)
|
||||
self.record_outcome(True)
|
||||
except RequestException as e:
|
||||
self.record_outcome(False, e.response)
|
||||
self.record_outcome(False)
|
||||
raise FiretextClientResponseException(response=e.response, exception=e)
|
||||
finally:
|
||||
elapsed_time = monotonic() - start_time
|
||||
|
||||
@@ -77,22 +77,6 @@ class MMGClient(SmsClient):
|
||||
self.statsd_client = statsd_client
|
||||
self.mmg_url = current_app.config.get('MMG_URL')
|
||||
|
||||
def record_outcome(self, success, response):
|
||||
status_code = response.status_code if response else 503
|
||||
log_message = "API {} request {} on {} response status_code {}".format(
|
||||
"POST",
|
||||
"succeeded" if success else "failed",
|
||||
self.mmg_url,
|
||||
status_code
|
||||
)
|
||||
|
||||
if success:
|
||||
self.current_app.logger.info(log_message)
|
||||
self.statsd_client.incr("clients.mmg.success")
|
||||
else:
|
||||
self.statsd_client.incr("clients.mmg.error")
|
||||
self.current_app.logger.warning(log_message)
|
||||
|
||||
@property
|
||||
def name(self):
|
||||
return 'mmg'
|
||||
@@ -124,11 +108,11 @@ class MMGClient(SmsClient):
|
||||
try:
|
||||
json.loads(response.text)
|
||||
except (ValueError, AttributeError) as e:
|
||||
self.record_outcome(False, response)
|
||||
self.record_outcome(False)
|
||||
raise MMGClientResponseException(response=response, exception=e)
|
||||
self.record_outcome(True, response)
|
||||
self.record_outcome(True)
|
||||
except RequestException as e:
|
||||
self.record_outcome(False, e.response)
|
||||
self.record_outcome(False)
|
||||
raise MMGClientResponseException(response=e.response, exception=e)
|
||||
finally:
|
||||
elapsed_time = monotonic() - start_time
|
||||
|
||||
Reference in New Issue
Block a user