Skip to content

Commit

Permalink
chore: Use specific exceptions, instead of ValueError
Browse files Browse the repository at this point in the history
Sentry logs logger.error and logger.exception calls, so it's not necessary to call capture_exception (which was given incorrect arguments).

Sentry groups exceptions by stack trace, so the unique error messages should not be an issue.
https://docs.sentry.io/product/data-management-settings/event-grouping/
  • Loading branch information
jpmckinney committed Oct 18, 2023
1 parent 1307615 commit ffbe005
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 101 deletions.
12 changes: 3 additions & 9 deletions app/background_processes/application_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from app.core.settings import app_settings
from app.core.user_dependencies import sesClient
from app.db.session import get_db, transaction_session
from app.exceptions import SkippedAwardError
from app.schema import core
from app.utils import email_utility

Expand Down Expand Up @@ -115,15 +116,8 @@ def create_application(
# if application already exists
application = get_existing_application(award_borrower_identifier, session)
if application:
error_data = {
"legal_identifier": legal_identifier,
"source_contract_id": source_contract_id,
"application_id": application.id,
}

background_utils.raise_sentry_error(
f"Skipping Award - Application ID {application.id} already exists on for award {source_contract_id}",
error_data,
raise SkippedAwardError(
f"{application.id=} already exists for {legal_identifier=} {source_contract_id=}"
)

new_uuid: str = background_utils.generate_uuid(award_borrower_identifier)
Expand Down
5 changes: 2 additions & 3 deletions app/background_processes/awards_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from sqlalchemy.orm.session import Session

from app.db.session import get_db
from app.exceptions import SkippedAwardError
from app.schema.core import Award

from . import background_utils
Expand Down Expand Up @@ -168,9 +169,7 @@ def create_award(entry, session: Session, borrower_id=None, previous=False) -> A

# if award already exists
if get_existing_award(source_contract_id, session):
background_utils.raise_sentry_error(
f"Skipping Award [previous {previous}] - Already exists on database", entry
)
raise SkippedAwardError(f"[{previous=}] Award already exists for {entry=}")

new_award = create_new_award(source_contract_id, entry, borrower_id, previous)

Expand Down
15 changes: 0 additions & 15 deletions app/background_processes/background_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import uuid

import httpx
import sentry_sdk

from app.core.settings import app_settings

Expand All @@ -31,20 +30,6 @@ def is_valid_email(email: str) -> bool:
return False


def raise_sentry_error(message: str, payload: dict):
"""
Raise a Sentry error with the given message and payload.
:param message: The error message to be sent to Sentry.
:type message: str
:param payload: The payload to be sent along with the error message.
:type payload: dict
"""

sentry_sdk.capture_exception(message, payload)
raise ValueError(message)


def generate_uuid(string: str) -> str:
"""
Generate a UUID based on the given string.
Expand Down
124 changes: 50 additions & 74 deletions app/background_processes/colombia_data_access.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from datetime import datetime, timedelta

from app.core.settings import app_settings
from app.exceptions import SkippedAwardError

from . import background_utils

Expand Down Expand Up @@ -36,6 +37,9 @@ def create_new_award(
:rtype: dict
"""

proceso_de_compra = entry["proceso_de_compra"]
proveedor_adjudicado = entry["proveedor_adjudicado"]

new_award = {
"source_contract_id": source_contract_id,
"source_url": entry.get("urlproceso", {}).get("url", ""),
Expand All @@ -46,7 +50,7 @@ def create_new_award(
"contractperiod_enddate": entry.get("fecha_de_fin_del_contrato", None),
"procurement_method": entry.get("modalidad_de_contratacion", ""),
"buyer_name": entry.get("nombre_entidad", ""),
"contracting_process_id": entry.get("proceso_de_compra", ""),
"contracting_process_id": proceso_de_compra,
"procurement_category": entry.get("tipo_de_contrato", ""),
"previous": previous,
"payment_method": {
Expand All @@ -60,39 +64,28 @@ def create_new_award(
}

award_url = (
f"{URLS['AWARDS']}?$where=id_del_portafolio='{entry['proceso_de_compra']}'"
f" AND nombre_del_proveedor='{entry['proveedor_adjudicado']}'"
f"{URLS['AWARDS']}?$where=id_del_portafolio='{proceso_de_compra}'"
f" AND nombre_del_proveedor='{proveedor_adjudicado}'"
)

award_response = background_utils.make_request_with_retry(award_url, headers)
award_response_json = award_response.json()
len_award_response_json = len(award_response_json)

if len(award_response.json()) > 1 or len(award_response.json()) == 0:
error_data = {
"entry": entry,
"proveedor_adjudicado": entry["proveedor_adjudicado"],
"id_del_portafolio": entry["proceso_de_compra"],
"response": award_response.json(),
}
background_utils.raise_sentry_error(
(
f"Skipping Award [previous {previous}]"
" - Zero or more than one results for 'proceso_de_compra' and 'proveedor_adjudicado'"
),
error_data,
if len_award_response_json != 1:
raise SkippedAwardError(
f"[{previous=}] {len_award_response_json} results for {proceso_de_compra=} and {proveedor_adjudicado=} "
f"({entry=} response={award_response_json})"
)

award_response_json = award_response.json()[0]
remote_award = award_response_json[0]

new_award["description"] = award_response_json.get(
"descripci_n_del_procedimiento", ""
)
new_award["award_date"] = award_response_json.get("fecha_adjudicacion", None)
new_award["source_data_awards"] = award_response_json
new_award["description"] = remote_award.get("descripci_n_del_procedimiento", "")
new_award["award_date"] = remote_award.get("fecha_adjudicacion", None)
new_award["source_data_awards"] = remote_award

new_award["contract_status"] = award_response_json.get(
"estado_del_procedimiento", ""
)
new_award["title"] = award_response_json.get("nombre_del_procedimiento", "")
new_award["contract_status"] = remote_award.get("estado_del_procedimiento", "")
new_award["title"] = remote_award.get("nombre_del_procedimiento", "")

if borrower_id:
new_award["borrower_id"] = borrower_id
Expand Down Expand Up @@ -161,7 +154,7 @@ def get_source_contract_id(entry):
source_contract_id = entry.get("id_contrato", "")

if not source_contract_id:
background_utils.raise_sentry_error("Skipping Award - No id_contrato", entry)
raise SkippedAwardError(f"No id_contrato in {entry=}")

return source_contract_id

Expand All @@ -188,34 +181,31 @@ def create_new_borrower(
f"&codigo_entidad={entry.get('codigo_proveedor', '')}"
)
borrower_response = background_utils.make_request_with_retry(borrower_url, headers)
borrower_response_json = borrower_response.json()
len_borrower_response_json = len(borrower_response_json)

if len(borrower_response.json()) > 1:
error_data = {
"entry": entry,
"documento_proveedor": documento_proveedor,
"response": borrower_response.json(),
}
background_utils.raise_sentry_error(
"Skipping Award - There are more than one borrower for this borrower identifier",
error_data,
if len_borrower_response_json > 1:
raise SkippedAwardError(
f"{len_borrower_response_json} borrowers for {documento_proveedor=} "
f"({entry=} response={borrower_response_json})"
)

borrower_response_json = borrower_response.json()[0]
remote_borrower = borrower_response_json[0]
email = get_email(documento_proveedor, entry)

new_borrower = {
"borrower_identifier": borrower_identifier,
"legal_name": borrower_response_json.get("nombre_entidad", ""),
"legal_name": remote_borrower.get("nombre_entidad", ""),
"email": email,
"address": "Direccion: {}\nCiudad: {}\nProvincia: {}\nEstado: {}".format(
borrower_response_json.get("direccion", "No provisto"),
borrower_response_json.get("ciudad", "No provisto"),
borrower_response_json.get("provincia", "No provisto"),
borrower_response_json.get("estado", "No provisto"),
remote_borrower.get("direccion", "No provisto"),
remote_borrower.get("ciudad", "No provisto"),
remote_borrower.get("provincia", "No provisto"),
remote_borrower.get("estado", "No provisto"),
),
"legal_identifier": borrower_response_json.get("nit_entidad", ""),
"type": borrower_response_json.get("tipo_organizacion", ""),
"source_data": borrower_response_json,
"legal_identifier": remote_borrower.get("nit_entidad", ""),
"type": remote_borrower.get("tipo_organizacion", ""),
"source_data": remote_borrower,
}

return new_borrower
Expand All @@ -238,42 +228,33 @@ def get_email(documento_proveedor, entry) -> str:
borrower_response_email = background_utils.make_request_with_retry(
borrower_email_url, headers
)
borrower_response_email_json = borrower_response_email.json()
len_borrower_response_email_json = len(borrower_response_email_json)

if len(borrower_response_email.json()) == 0:
error_data = {
"entry": entry,
"response": borrower_response_email.json(),
}
background_utils.raise_sentry_error(
"Skipping Award - No email for borrower", error_data
if len_borrower_response_email_json == 0:
raise SkippedAwardError(
f"0 emails for borrower ({entry=} response={borrower_response_email_json})"
)

borrower_response_email_json = borrower_response_email.json()[0]
email = borrower_response_email_json.get("correo_entidad", "")
remote_email = borrower_response_email_json[0]
email = remote_email.get("correo_entidad", "")

if not background_utils.is_valid_email(email):
error_data = {
"entry": entry,
"response": borrower_response_email_json,
}
background_utils.raise_sentry_error(
"Skipping Award - Borrower has no valid email address", error_data
raise SkippedAwardError(
f"First email for borrower is invalid ({remote_email=} {entry=})"
)

if len(borrower_response_email.json()) > 1:
if len_borrower_response_email_json > 1:
same_email = True
for borrower_email in borrower_response_email.json():
for borrower_email in borrower_response_email_json:
if borrower_email.get("correo_entidad", "") != email:
same_email = False
break

if not same_email:
error_data = {
"entry": entry,
"response": borrower_response_email.json(),
}
background_utils.raise_sentry_error(
"Skipping Award - More than one email for borrower", error_data
raise SkippedAwardError(
f"{len_borrower_response_email_json} emails for borrower "
f"({entry=} response={borrower_response_email_json})"
)

return email
Expand All @@ -292,11 +273,6 @@ def get_documento_proveedor(entry) -> str:

documento_proveedor = entry.get("documento_proveedor", None)
if not documento_proveedor or documento_proveedor == "No Definido":
error_data = {"entry": entry}

background_utils.raise_sentry_error(
"Skipping Award - documento_proveedor is 'No Definido'",
error_data,
)
raise SkippedAwardError(f"documento_proveedor is 'No Definido' ({entry=})")

return documento_proveedor
5 changes: 5 additions & 0 deletions app/db/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from sqlalchemy.orm.session import Session

from app.core.settings import app_settings
from app.exceptions import SkippedAwardError

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -43,6 +44,10 @@ def transaction_session_logger(session: Session, msg: str, *args):
try:
yield session
session.commit()
# Don't display tracebacks in emails from cron jobs for anticipated errors.
except SkippedAwardError:
logger.error(msg, *args)
session.rollback()
except Exception:
logger.exception(msg, *args)
session.rollback()
Expand Down
6 changes: 6 additions & 0 deletions app/exceptions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class CredereError(Exception):
"""Base class for exceptions from within this application"""


class SkippedAwardError(CredereError):
"""Raised if an award needs to be skipped due to a data quality issue"""

0 comments on commit ffbe005

Please sign in to comment.