Skip to content

Commit 77e7300

Browse files
mn-ramclaude
andcommitted
[fix] Add logging and regression tests for update_config self-detection
Added logger.info when update_config is skipped due to another active task, improving traceability. Added regression tests to verify that _is_update_in_progress correctly excludes the calling task's own ID while still detecting other active tasks. Fixed Black formatting. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 41bc1c9 commit 77e7300

File tree

2 files changed

+42
-4
lines changed

2 files changed

+42
-4
lines changed

openwisp_controller/connection/tasks.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,9 @@ def update_config(self, device_id):
5353
logger.warning(f'update_config("{device_id}") failed: {e}')
5454
return
5555
if _is_update_in_progress(device_id, current_task_id=self.request.id):
56+
logger.info(
57+
f"update_config for device {device_id} is already in progress, skipping"
58+
)
5659
return
5760
try:
5861
device_conn = DeviceConnection.get_working_connection(device)

openwisp_controller/connection/tests/test_models.py

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
)
2222
from ..exceptions import NoWorkingDeviceConnectionError
2323
from ..signals import is_working_changed
24-
from ..tasks import _TASK_NAME, update_config
24+
from ..tasks import _TASK_NAME, _is_update_in_progress, update_config
2525
from .utils import CreateConnectionsMixin
2626

2727
Config = load_model("config", "Config")
@@ -1060,9 +1060,7 @@ def test_device_update_config_not_in_progress(
10601060

10611061
with mock.patch("celery.app.control.Inspect.active") as mocked_active:
10621062
mocked_active.return_value = {
1063-
"task": [
1064-
{"name": _TASK_NAME, "args": ["..."], "id": "other-task-id"}
1065-
]
1063+
"task": [{"name": _TASK_NAME, "args": ["..."], "id": "other-task-id"}]
10661064
}
10671065
conf.config = {"general": {"timezone": "UTC"}}
10681066
conf.full_clean()
@@ -1071,6 +1069,43 @@ def test_device_update_config_not_in_progress(
10711069
mocked_get_working_connection.assert_called_once()
10721070
mocked_update_config.assert_called_once()
10731071

1072+
def test_is_update_in_progress_ignores_current_task(self):
1073+
"""Regression test: _is_update_in_progress must not count
1074+
the calling task itself as a duplicate."""
1075+
device_id = "test-device-id"
1076+
current_task_id = "current-task-id"
1077+
with mock.patch("celery.app.control.Inspect.active") as mocked_active:
1078+
mocked_active.return_value = {
1079+
"worker": [
1080+
{
1081+
"name": _TASK_NAME,
1082+
"args": [device_id],
1083+
"id": current_task_id,
1084+
}
1085+
]
1086+
}
1087+
result = _is_update_in_progress(device_id, current_task_id=current_task_id)
1088+
self.assertFalse(result)
1089+
1090+
def test_is_update_in_progress_detects_other_task(self):
1091+
"""_is_update_in_progress returns True when another task
1092+
for the same device is active."""
1093+
device_id = "test-device-id"
1094+
with mock.patch("celery.app.control.Inspect.active") as mocked_active:
1095+
mocked_active.return_value = {
1096+
"worker": [
1097+
{
1098+
"name": _TASK_NAME,
1099+
"args": [device_id],
1100+
"id": "other-task-id",
1101+
}
1102+
]
1103+
}
1104+
result = _is_update_in_progress(
1105+
device_id, current_task_id="current-task-id"
1106+
)
1107+
self.assertTrue(result)
1108+
10741109
@mock.patch(_connect_path)
10751110
def test_schedule_command_called(self, connect_mocked):
10761111
dc = self._create_device_connection()

0 commit comments

Comments
 (0)