darkhelm/notifications-admin
1
0
Fork 0
mirror of https://github.com/GSA/notifications-admin.git synced 2026-09-06 12:48:25 -04:00
Code Issues Packages Projects Releases Wiki Activity

Bugfix/polling improvements - Pointing polling to new backend endpoint designed for status only (#2949)

Browse Source 
* Optimizing polling

* Fixed formatting issue

* Pointed new endpoint to new backend polling endpoint

* Missed a unit test update

* Fixed flake issue
This commit is contained in:
Alex Janousek
2025-09-26 14:51:29 -04:00
committed by GitHub
parent 60b7f9149e
commit bc474c9206
6 changed files with 67 additions and 103 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
app/__init__.py
View File

@@ -199,7 +199,7 @@ def create_app(application):
current_app.logger.info( current_app.logger.info(
f"FEATURE_SOCKET_ENABLED value in __init__.py coming \ f"FEATURE_SOCKET_ENABLED value in __init__.py coming \
from config is {application.config.get('FEATURED_SOCKET_ENABLED')} and \ from config is {application.config.get('FEATURE_SOCKET_ENABLED')} and \
the ending value is {feature_socket_enabled}" the ending value is {feature_socket_enabled}"
) )
return dict( return dict(

1
app/assets/javascripts/job-status-polling.js
View File

@@ -126,7 +126,6 @@ document.addEventListener('DOMContentLoaded', function () {
await updateNotifications(); await updateNotifications();
stopPolling(); stopPolling();
} }
} catch (error) { } catch (error) {
if (retryCount < 3) { if (retryCount < 3) {
console.debug(`Job polling retry ${retryCount}`, error.message); console.debug(`Job polling retry ${retryCount}`, error.message);

38
app/main/views/jobs.py
View File

@@ -1,6 +1,7 @@
# -*- coding: utf-8 -*- # -*- coding: utf-8 -*-
import json import json
import os import os
import time
from functools import partial from functools import partial
from flask import ( from flask import (
@@ -21,6 +22,7 @@ from markupsafe import Markup
from app import ( from app import (
current_service, current_service,
format_datetime_table, format_datetime_table,
job_api_client,
notification_api_client, notification_api_client,
service_api_client, service_api_client,
) )
@@ -111,25 +113,33 @@ def cancel_job(service_id, job_id):
@user_has_permissions() @user_has_permissions()
def view_job_status_poll(service_id, job_id): def view_job_status_poll(service_id, job_id):
""" """
Poll status endpoint that only queries jobs table. Poll status endpoint using new lightweight cached API endpoint.
Returns minimal data needed for polling. Returns minimal data needed for polling.
""" """
import time
start_time = time.time() start_time = time.time()
job = Job.from_id(job_id, service_id=service_id) # Use new lightweight status endpoint
try:
status_data = job_api_client.get_job_status(service_id, job_id)
processed_count = job.notifications_delivered + job.notifications_failed # Validate the response has expected fields
total_count = job.notification_count required_fields = ["sent_count", "failed_count", "pending_count", "total_count", "processing_finished"]
if all(key in status_data for key in required_fields):
response_data = {
"sent_count": status_data["sent_count"],
"failed_count": status_data["failed_count"],
"pending_count": status_data["pending_count"],
"total_count": status_data["total_count"],
"finished": status_data["processing_finished"],
}
processed_count = status_data["sent_count"] + status_data["failed_count"]
else:
current_app.logger.error(f"Status endpoint returned invalid response for job {job_id[:8]}: {status_data}")
abort(500, "Invalid status response from API")
response_data = { except Exception as e:
"sent_count": job.notifications_delivered, current_app.logger.error(f"Status endpoint failed for job {job_id[:8]}: {e}")
"failed_count": job.notifications_failed, abort(500, "Status endpoint unavailable")
"pending_count": job.notifications_sending,
"total_count": total_count,
"finished": job.finished_processing,
}
response_time_ms = round((time.time() - start_time) * 1000, 2) response_time_ms = round((time.time() - start_time) * 1000, 2)
response_json = json.dumps(response_data) response_json = json.dumps(response_data)
@@ -139,7 +149,7 @@ def view_job_status_poll(service_id, job_id):
f"Poll status request - job_id={job_id[:8]} " f"Poll status request - job_id={job_id[:8]} "
f"response_size={response_size_bytes}b " f"response_size={response_size_bytes}b "
f"response_time={response_time_ms}ms " f"response_time={response_time_ms}ms "
f"progress={processed_count}/{total_count}" f"progress={processed_count}/{response_data['total_count']}"
) )
return jsonify(response_data) return jsonify(response_data)

3
app/notify_client/job_api_client.py
View File

@@ -32,6 +32,9 @@ class JobApiClient(NotifyAdminAPIClient):
return job return job
def get_job_status(self, service_id, job_id):
return self.get(url=f"/service/{service_id}/job/{job_id}/status")
def get_jobs( def get_jobs(
self, self,
service_id, service_id,

51
tests/app/main/views/test_job_notification_updates.py
View File

@@ -12,8 +12,6 @@ import json
import pytest import pytest
from tests import job_json, user_json
@pytest.mark.parametrize( @pytest.mark.parametrize(
("delivered", "failed", "pending", "finished", "js_should_update_notifications", "reason"), ("delivered", "failed", "pending", "finished", "js_should_update_notifications", "reason"),
@@ -46,25 +44,14 @@ def test_poll_status_notification_update_logic(
job is finished (regardless of count) job is finished (regardless of count)
""" """
total = delivered + failed + pending total = delivered + failed + pending
job_status = "finished" if finished else "sending"
mock_job = mocker.patch("app.job_api_client.get_job") mock_job_status = mocker.patch("app.job_api_client.get_job_status")
mock_job.return_value = { mock_job_status.return_value = {
"data": { "sent_count": delivered,
**job_json( "failed_count": failed,
service_one["id"], "pending_count": pending,
created_by=user_json(), "total_count": total,
job_id=fake_uuid, "processing_finished": finished,
job_status=job_status,
notification_count=total,
notifications_requested=total,
),
"statistics": [
{"status": "delivered", "count": delivered},
{"status": "failed", "count": failed},
{"status": "pending", "count": pending},
],
}
} }
response = client_request.get_response( response = client_request.get_response(
@@ -107,23 +94,13 @@ def test_poll_status_provides_required_fields(
fake_uuid, fake_uuid,
): ):
"""Verify poll status endpoint returns all fields needed for notification update logic.""" """Verify poll status endpoint returns all fields needed for notification update logic."""
mock_job = mocker.patch("app.job_api_client.get_job") mock_job_status = mocker.patch("app.job_api_client.get_job_status")
mock_job.return_value = { mock_job_status.return_value = {
"data": { "sent_count": 15,
**job_json( "failed_count": 5,
service_one["id"], "pending_count": 5,
created_by=user_json(), "total_count": 25,
job_id=fake_uuid, "processing_finished": False,
job_status="sending",
notification_count=25,
notifications_requested=25,
),
"statistics": [
{"status": "delivered", "count": 15},
{"status": "failed", "count": 5},
{"status": "pending", "count": 5},
],
}
} }
response = client_request.get_response( response = client_request.get_response(

75
tests/app/main/views/test_jobs.py
View File

@@ -510,23 +510,13 @@ def test_poll_status_endpoint(
fake_uuid, fake_uuid,
): ):
"""Test that the poll status endpoint returns only required data without notifications""" """Test that the poll status endpoint returns only required data without notifications"""
mock_job = mocker.patch("app.job_api_client.get_job") mock_job_status = mocker.patch("app.job_api_client.get_job_status")
mock_job.return_value = { mock_job_status.return_value = {
"data": { "sent_count": 90,
**job_json( "failed_count": 10,
service_one["id"], "pending_count": 0,
created_by=user_json(), "total_count": 100,
job_id=fake_uuid, "processing_finished": True,
job_status="finished",
notification_count=100,
notifications_requested=100,
),
"statistics": [
{"status": "delivered", "count": 90},
{"status": "failed", "count": 10},
{"status": "pending", "count": 0},
],
}
} }
response = client_request.get_response( response = client_request.get_response(
@@ -563,19 +553,13 @@ def test_poll_status_with_zero_notifications(
fake_uuid, fake_uuid,
): ):
"""Test poll status endpoint handles edge case of no notifications""" """Test poll status endpoint handles edge case of no notifications"""
mock_job = mocker.patch("app.job_api_client.get_job") mock_job_status = mocker.patch("app.job_api_client.get_job_status")
mock_job.return_value = { mock_job_status.return_value = {
"data": { "sent_count": 0,
**job_json( "failed_count": 0,
service_one["id"], "pending_count": 0,
created_by=user_json(), "total_count": 0,
job_id=fake_uuid, "processing_finished": True,
job_status="pending",
notification_count=0,
notifications_requested=0,
),
"statistics": [],
}
} }
response = client_request.get_response( response = client_request.get_response(
@@ -588,9 +572,7 @@ def test_poll_status_with_zero_notifications(
data = json.loads(response.get_data(as_text=True)) data = json.loads(response.get_data(as_text=True))
assert data["total_count"] == 0 assert data["total_count"] == 0
assert ( assert data["finished"] is True
data["finished"] is True
)
def test_poll_status_endpoint_does_not_query_notifications_table( def test_poll_status_endpoint_does_not_query_notifications_table(
@@ -602,23 +584,13 @@ def test_poll_status_endpoint_does_not_query_notifications_table(
fake_uuid, fake_uuid,
): ):
"""Critical regression test: ensure poll status endpoint never queries notifications""" """Critical regression test: ensure poll status endpoint never queries notifications"""
mock_job = mocker.patch("app.job_api_client.get_job") mock_job_status = mocker.patch("app.job_api_client.get_job_status")
mock_job.return_value = { mock_job_status.return_value = {
"data": { "sent_count": 300,
**job_json( "failed_count": 50,
service_one["id"], "pending_count": 150,
created_by=user_json(), "total_count": 500,
job_id=fake_uuid, "processing_finished": False,
job_status="sending",
notification_count=500,
notifications_requested=500,
),
"statistics": [
{"status": "delivered", "count": 300},
{"status": "failed", "count": 50},
{"status": "pending", "count": 150},
],
}
} }
mock_get_notifications = mocker.patch( mock_get_notifications = mocker.patch(
@@ -639,3 +611,6 @@ def test_poll_status_endpoint_does_not_query_notifications_table(
data = json.loads(response.get_data(as_text=True)) data = json.loads(response.get_data(as_text=True))
assert data["total_count"] == 500 assert data["total_count"] == 500
assert data["sent_count"] == 300 assert data["sent_count"] == 300
assert data["failed_count"] == 50
assert data["pending_count"] == 150
assert data["finished"] is False