chore: Remove info-level logging that is unlikely to ever be read, as…

… it's run in web context
open-contracting · Apr 2, 2024 · 1d4febb · 1d4febb
1 parent bbeb639
commit 1d4febb
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 60 deletions.
diff --git a/app/util.py b/app/util.py
@@ -1,7 +1,6 @@
 import base64
 import hashlib
 import hmac
-import logging
 import os.path
 import uuid
 from contextlib import contextmanager
@@ -22,8 +21,6 @@
 MAX_FILE_SIZE = app_settings.max_file_size_mb * 1024 * 1024  # MB in bytes
 ALLOWED_EXTENSIONS = {".png", ".pdf", ".jpeg", ".jpg"}
 
-logger = logging.getLogger(__name__)
-
 
 class ERROR_CODES(StrEnum):
     BORROWER_FIELD_VERIFICATION_MISSING = "BORROWER_FIELD_VERIFICATION_MISSING"
@@ -174,12 +171,8 @@ def get_previous_awards_from_data_source(
     contracts_response = data_access.get_previous_contracts(borrower.legal_identifier)
     contracts_response_json = contracts_response.json()
     if not contracts_response_json:
-        logger.info("No previous contracts for %s", borrower.legal_identifier)
         return
 
-    logger.info(
-        "Previous contracts for %s response length: %s", borrower.legal_identifier, len(contracts_response_json)
-    )
     for entry in contracts_response_json:
         with contextmanager(db_provider)() as session:
             with handle_skipped_award(session, f"Error creating the previous award for {borrower.legal_identifier}"):

diff --git a/app/utils/statistics.py b/app/utils/statistics.py
@@ -1,4 +1,3 @@
-import logging
 from contextlib import contextmanager
 from datetime import datetime
 from typing import Any, Callable, Generator
@@ -22,8 +21,6 @@
     StatisticType,
 )
 
-logger = logging.getLogger(__name__)
-
 keys_to_serialize = [
     "sector_statistics",
     "rejected_reasons_count_by_reason",
@@ -50,21 +47,13 @@ def update_statistics(db_provider: Callable[[], Generator[Session, None, None]]
 
     This function retrieves and logs different types of statistics related to applications
     and lenders. It uses the `get_general_statistics` and `get_msme_opt_in_stats` functions
-    to fetch the respective statistics. The retrieved statistics are then logged using
-    the `logger.info()` function.
+    to fetch the respective statistics.
 
     After fetching the general statistics, this function attempts to store them in the database
     as an instance of the `Statistic` model. The statistics are stored with the type set to
     `StatisticType.APPLICATION_KPIS`. The `Statistic` model contains a JSON field to store
     the actual statistical data.
 
-    If an error occurs during the process, it is caught and logged using `logger.exception()`.
-    The database session is rolled back in case of an exception to prevent any changes from
-    being committed to the database.
-
-    Note:
-    - The function utilizes the `get_db()` context manager to open a database session.
-
     Example usage:
     >>> update_statistics()
     """
@@ -318,7 +307,6 @@ def get_msme_opt_in_stats(session: Session) -> dict[str, Any]:
     """
 
     WOMAN_VALUES = ("Femenino", "Mujer")
-    logger.info("calculating msme opt in stats for OCP ")
 
     unique_smes_contacted_by_credere = session.query(Borrower).count()
 

diff --git a/tests/test_fetcher.py b/tests/test_fetcher.py
@@ -71,6 +71,7 @@ def _mock_whole_process(status_code: int, award_mock: dict, borrower_mock: dict,
             MockResponse(status_code, award_mock),
             MockResponse(status_code, borrower_mock),
             MockResponse(status_code, email_mock),
+            MockResponse(status_code, award_mock),
         ]
     )
 
@@ -123,50 +124,54 @@ def _compare_objects(
         assert expected_result[key] == value
 
 
-def test_fetch_previous_borrower_awards_empty(engine, create_and_drop_database, caplog):
-    with caplog.at_level("INFO"):
-        with _mock_response(
-            200,
-            [],
-            "app.sources.colombia.get_previous_contracts",
-        ), _mock_response(
-            200,
-            contract,
-            "app.sources.colombia.get_contract_by_contract_and_supplier",
-        ), _mock_whole_process(
-            200,
-            award,
-            borrower,
-            email,
-            "app.sources.make_request_with_retry",
-        ):
-            fetch_award_by_contract_and_supplier(CONTRACT_ID, SUPPLIER_ID)
-            util.get_previous_awards_from_data_source(borrower_result["id"], get_test_db(engine))
+def test_fetch_previous_borrower_awards_empty(engine, create_and_drop_database):
+    with contextmanager(get_test_db(engine))() as session, _mock_response(
+        200,
+        [],  # changed
+        "app.sources.colombia.get_previous_contracts",
+    ), _mock_response(
+        200,
+        contract,
+        "app.sources.colombia.get_contract_by_contract_and_supplier",
+    ), _mock_whole_process(
+        200,
+        award,
+        borrower,
+        email,
+        "app.sources.make_request_with_retry",
+    ):
+        fetch_award_by_contract_and_supplier(CONTRACT_ID, SUPPLIER_ID)
+        util.get_previous_awards_from_data_source(borrower_result["id"], get_test_db(engine))
 
-    assert "No previous contracts" in caplog.text
+        assert session.query(models.Award).count() == 1
+        assert session.query(models.EventLog).count() == 0, session.query(models.EventLog).one()
 
 
-def test_fetch_previous_borrower_awards(engine, create_and_drop_database, caplog):
-    with caplog.at_level("INFO"):
-        with _mock_response(
-            200,
-            [{"key": "value"}],
-            "app.sources.colombia.get_previous_contracts",
-        ), _mock_response(
-            200,
-            contract,
-            "app.sources.colombia.get_contract_by_contract_and_supplier",
-        ), _mock_whole_process(
-            200,
-            award,
-            borrower,
-            email,
-            "app.sources.make_request_with_retry",
-        ):
-            fetch_award_by_contract_and_supplier(CONTRACT_ID, SUPPLIER_ID)
-            util.get_previous_awards_from_data_source(borrower_result["id"], get_test_db(engine))
+def test_fetch_previous_borrower_awards(engine, create_and_drop_database):
+    # We can make a shallow copy, as we change `id_contrato` only, to make `source_contract_id` different.
+    previous_contract = contract[0].copy()
+    previous_contract["id_contrato"] = "CO1.test.123456.previous"
+
+    with contextmanager(get_test_db(engine))() as session, _mock_response(
+        200,
+        [previous_contract],  # changed
+        "app.sources.colombia.get_previous_contracts",
+    ), _mock_response(
+        200,
+        contract,
+        "app.sources.colombia.get_contract_by_contract_and_supplier",
+    ), _mock_whole_process(
+        200,
+        award,
+        borrower,
+        email,
+        "app.sources.make_request_with_retry",
+    ):
+        fetch_award_by_contract_and_supplier(CONTRACT_ID, SUPPLIER_ID)
+        util.get_previous_awards_from_data_source(borrower_result["id"], get_test_db(engine))
 
-    assert "Previous contracts for" in caplog.text
+        assert session.query(models.Award).count() == 2
+        assert session.query(models.EventLog).count() == 0, session.query(models.EventLog).one()
 
 
 def test_fetch_empty_contracts(create_and_drop_database, caplog):