Merge pull request #4110 from alphagov/revert-service-id-logging-commit

Revert "Log service ID as part of sending callbacks if possible"

app/celery/service_callback_tasks.py

@@ -75,21 +75,14 @@ def send_inbound_sms_to_service(self, inbound_sms_id, service_id):
     }
 
     _send_data_to_service_callback_api(
-        self,
-        data,
-        inbound_api.url,
-        inbound_api.bearer_token,
-        "send_inbound_sms_to_service",
-        QueueNames.RETRY,
-        service_id,
+        self, data, inbound_api.url, inbound_api.bearer_token, "send_inbound_sms_to_service", QueueNames.RETRY
     )
 
 
 def _send_data_to_service_callback_api(
-    self, data, service_callback_url, token, function_name, retry_queue=QueueNames.CALLBACKS_RETRY, service_id=None
+    self, data, service_callback_url, token, function_name, retry_queue=QueueNames.CALLBACKS_RETRY
 ):
     object_id = data["notification_id"] if "notification_id" in data else data["id"]
-    service_id = str(service_id) if service_id else "no-service-id"
     try:
         response = request(
             method="POST",
@@ -104,7 +97,6 @@ def _send_data_to_service_callback_api(
             object_id,
             service_callback_url,
             response.status_code,
-            extra={"service_id": service_id},
         )
         response.raise_for_status()
     except RequestException as e:
@@ -114,7 +106,6 @@ def _send_data_to_service_callback_api(
             object_id,
             service_callback_url,
             e,
-            extra={"service_id": service_id},
         )
         if not isinstance(e, HTTPError) or e.response.status_code >= 500 or e.response.status_code == 429:
             try:
@@ -125,7 +116,6 @@ def _send_data_to_service_callback_api(
                     function_name,
                     service_callback_url,
                     object_id,
-                    extra={"service_id": service_id},
                 )
         else:
             current_app.logger.warning(
@@ -134,7 +124,6 @@ def _send_data_to_service_callback_api(
                 object_id,
                 service_callback_url,
                 e,
-                extra={"service_id": service_id},
             )
 
 

tests/app/celery/test_service_callback_tasks.py

@@ -154,13 +154,7 @@ def test_send_inbound_sms_to_service_sends_callback_to_service(notify_api, sampl
 
     send_inbound_sms_to_service(inbound_sms.id, inbound_sms.service_id)
     send_callback_mock.assert_called_once_with(
-        mock.ANY,
-        data,
-        "https://some.service.gov.uk/",
-        "something_unique",
-        "send_inbound_sms_to_service",
-        "retry-tasks",
-        sample_service.id,
+        mock.ANY, data, "https://some.service.gov.uk/", "something_unique", "send_inbound_sms_to_service", "retry-tasks"
     )
 
 
@@ -263,14 +257,3 @@ def test__send_data_to_service_callback_api_handles_data_with_notification_id_or
     with requests_mock.Mocker() as request_mock:
         request_mock.post(callback_url, json={}, status_code=200)
         _send_data_to_service_callback_api(mock.MagicMock(), data, callback_url, "my-token", "my_function_name")
-
-
-@pytest.mark.parametrize("service_id", ["string-service-id", uuid.uuid4()])
-def test__send_data_to_service_callback_api_handles_service_id(notify_db_session, mocker, service_id):
-    callback_url = "https://www.example.com/callback"
-
-    with requests_mock.Mocker() as request_mock:
-        request_mock.post(callback_url, json={}, status_code=200)
-        _send_data_to_service_callback_api(
-            mock.MagicMock(), {"id": "hello"}, callback_url, "my-token", "my_function_name", service_id=service_id
-        )

tests/app/celery/test_tasks.py

@@ -1438,7 +1438,7 @@ def test_process_incomplete_jobs_no_notifications_added(mocker, sample_template)
     assert mock_save_sms.call_count == 10  # There are 10 in the csv file
 
 
-def test_process_incomplete_jobs(mocker):
+def test_process_incomplete_jobs(mocker, sample_template):
     mocker.patch(
         "app.celery.tasks.s3.get_job_and_metadata_from_s3",
         return_value=(load_example_csv("multiple_sms"), {"sender_id": None}),