operation: Add unique ID for each operation

- Helps in retrieving an operation that is currently running.

- Prevent starting an operation that is already running.

Tests:

- Unit tests work.

- Installing, uninstalling an app works.

- For upgrading an app works.

- Running background diagnostics works.

- Updating tor configuration works.

- Updating torproxy configuration works.

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>
This commit is contained in:
Sunil Mohan Adapa 2023-10-02 18:45:21 -07:00 committed by James Valleroy
parent 79f36e6a0c
commit a233bbfd9b
No known key found for this signature in database
GPG Key ID: 77C0C75E7B650808
6 changed files with 85 additions and 48 deletions

View File

@ -264,8 +264,9 @@ def _warn_about_low_ram_space(request):
def _start_background_diagnostics(request):
"""Start daily diagnostics as a background operation."""
operation = operation_module.manager.new(
'diagnostics', gettext_noop('Running background diagnostics'),
_run_background_diagnostics, [], show_message=False,
op_id='diagnostics-full', app_id='diagnostics',
name=gettext_noop('Running background diagnostics'),
target=_run_background_diagnostics, [], show_message=False,
show_notification=False)
operation.join()

View File

@ -49,7 +49,7 @@ class TorAppView(AppView):
def form_valid(self, form):
"""Configure tor app on successful form submission."""
operation_module.manager.new(self.app_id,
operation_module.manager.new('tor-configuration', self.app_id,
gettext_noop('Updating configuration'),
_apply_changes,
[form.initial, form.cleaned_data],

View File

@ -47,7 +47,7 @@ class TorProxyAppView(AppView):
def form_valid(self, form):
"""Configure tor app on successful form submission."""
operation_module.manager.new(self.app_id,
operation_module.manager.new('torproxy-configuration', self.app_id,
gettext_noop('Updating configuration'),
_apply_changes,
[form.initial, form.cleaned_data],

View File

@ -4,6 +4,7 @@
import enum
import logging
import threading
from collections import OrderedDict
from typing import Callable
from . import app as app_module
@ -21,12 +22,13 @@ class Operation:
RUNNING: str = 'running'
COMPLETED: str = 'completed'
def __init__(self, app_id: str, name: str, target: Callable,
def __init__(self, op_id: str, app_id: str, name: str, target: Callable,
args: list | None = None, kwargs: dict | None = None,
show_message: bool = True, show_notification: bool = False,
thread_data: dict | None = None,
on_complete: Callable | None = None):
"""Initialize to no operation."""
self.op_id = op_id
self.app_id = app_id
self.name = name
self.show_message = show_message
@ -171,7 +173,7 @@ class OperationsManager:
def __init__(self) -> None:
"""Initialize the object."""
self._operations: list[Operation] = []
self._operations: OrderedDict[str, Operation] = OrderedDict()
self._current_operation: Operation | None = None
# Assume that operations manager will be called from various threads
@ -182,16 +184,24 @@ class OperationsManager:
# when done from the same thread which holds the lock.
self._lock = threading.RLock()
def new(self, *args, **kwargs) -> Operation:
def new(self, op_id: str, *args, **kwargs) -> Operation:
"""Create a new operation instance and add to global list."""
with self._lock:
if op_id in self._operations:
raise KeyError('Operation in progress/scheduled')
kwargs['on_complete'] = self._on_operation_complete
operation = Operation(*args, **kwargs)
self._operations.append(operation)
operation = Operation(op_id, *args, **kwargs)
self._operations[op_id] = operation
logger.info('%s: added', operation)
self._schedule_next()
return operation
def get(self, op_id: str) -> Operation:
"""Return an operation with given operation ID."""
with self._lock:
return self._operations[op_id]
def _on_operation_complete(self, operation: Operation):
"""Trigger next operation. Called from within previous thread."""
logger.debug('%s: on_complete called', operation)
@ -199,7 +209,7 @@ class OperationsManager:
self._current_operation = None
if not operation.show_message:
# No need to keep it lingering for later collection
self._operations.remove(operation)
del self._operations[operation.op_id]
self._schedule_next()
@ -209,7 +219,7 @@ class OperationsManager:
if self._current_operation:
return
for operation in self._operations:
for operation in self._operations.values():
if operation.state == Operation.State.WAITING:
logger.debug('%s: scheduling', operation)
self._current_operation = operation
@ -220,22 +230,22 @@ class OperationsManager:
"""Return operations matching a pattern."""
with self._lock:
return [
operation for operation in self._operations
operation for operation in self._operations.values()
if operation.app_id == app_id
]
def collect_results(self, app_id: str) -> list[Operation]:
"""Return the finished operations for an app."""
results: list[Operation] = []
remaining: list[Operation] = []
remaining: OrderedDict[str, Operation] = OrderedDict()
with self._lock:
for operation in self._operations:
for operation in self._operations.values():
if (operation.app_id == app_id
and operation.state == Operation.State.COMPLETED):
results.append(operation)
else:
remaining.append(operation)
remaining[operation.op_id] = operation
if results:
self._operations = remaining

View File

@ -46,8 +46,9 @@ def run_setup_on_app(app_id, allow_install=True, rerun=False):
show_notification = show_message = (current_version
or not app.info.is_essential)
return operation_module.manager.new(
app_id, name, _run_setup_on_app, [app, current_version],
show_message=show_message, show_notification=show_notification,
f'{app_id}-setup', app_id, name, _run_setup_on_app,
[app, current_version], show_message=show_message,
show_notification=show_notification,
thread_data={'allow_install': allow_install})
@ -101,7 +102,7 @@ def run_uninstall_on_app(app_id):
return
logger.debug('Creating operation to uninstall app: %s', app_id)
return operation_module.manager.new(app_id,
return operation_module.manager.new(f'{app_id}-uninstall', app_id,
gettext_noop('Uninstalling app'),
_run_uninstall_on_app, [app],
show_notification=True)
@ -450,7 +451,8 @@ class ForceUpgrader():
def _run_force_upgrade_as_operation(self, app, packages):
"""Start an operation for force upgrading."""
name = gettext_noop('Updating app packages')
operation = operation_module.manager.new(app.app_id, name,
operation = operation_module.manager.new(f'{app.app_id}-force-upgrade',
app.app_id, name,
app.force_upgrade, [packages],
show_message=False,
show_notification=False)

View File

@ -3,6 +3,7 @@
import threading
import time
from collections import OrderedDict
from unittest.mock import Mock, call, patch
import pytest
@ -28,7 +29,8 @@ class AppTest(app.App):
def test_operation_default_initialization(update_notification):
"""Test Operation initialization with default values."""
target = Mock()
operation = Operation('testapp', 'op1', target)
operation = Operation('testid', 'testapp', 'op1', target)
assert operation.op_id == 'testid'
assert operation.app_id == 'testapp'
assert operation.name == 'op1'
assert operation.show_message
@ -50,7 +52,7 @@ def test_operation_default_initialization(update_notification):
def test_operation_initialization(update_notification):
"""Test Operation initialization with explicit values."""
on_complete = Mock()
operation = Operation('testapp', 'op1', Mock(), ['arg1'],
operation = Operation('testid', 'testapp', 'op1', Mock(), ['arg1'],
{'arg2': 'value2'}, False, True,
{'data1': 'datavalue1'}, on_complete)
assert not operation.show_message
@ -67,7 +69,7 @@ def test_operation_initialization(update_notification):
def test_operation_str():
"""Test string representation of operation."""
operation = Operation('testapp', 'op1', Mock())
operation = Operation('testid', 'testapp', 'op1', Mock())
assert str(operation) == 'Operation: testapp: op1'
@ -77,7 +79,7 @@ def test_successful_operation(update_notification):
target = Mock()
target.return_value = 'test-return'
on_complete = Mock()
operation = Operation('testapp', 'op1', target, ['arg1'],
operation = Operation('testid', 'testapp', 'op1', target, ['arg1'],
{'arg2': 'value2'}, on_complete=on_complete)
operation.run()
assert operation.join() == 'test-return'
@ -94,7 +96,7 @@ def test_error_operation(update_notification):
target = Mock()
target.side_effect = RuntimeError('error1')
on_complete = Mock()
operation = Operation('testapp', 'op1', target, ['arg1'],
operation = Operation('testid', 'testapp', 'op1', target, ['arg1'],
{'arg2': 'value2'}, on_complete=on_complete)
operation.run()
with pytest.raises(RuntimeError):
@ -111,7 +113,7 @@ def test_error_operation(update_notification):
def test_join_before_start(update_notification):
"""Test waiting until operation finishes.."""
event = threading.Event()
operation = Operation('testapp', 'op1', Mock)
operation = Operation('testid', 'testapp', 'op1', Mock)
success = []
def _wait():
@ -135,7 +137,7 @@ def test_join_raises_exception(update_notification):
target = Mock()
target.side_effect = RuntimeError('error1')
on_complete = Mock()
operation = Operation('testapp', 'op1', target, ['arg1'],
operation = Operation('testid', 'testapp', 'op1', target, ['arg1'],
{'arg2': 'value2'}, on_complete=on_complete)
operation.run()
with pytest.raises(RuntimeError):
@ -149,7 +151,7 @@ def test_getting_operation_from_thread():
operation = Operation.get_operation()
operation.thread_data['test_operation'] = operation
operation = Operation('testapp', 'op1', target)
operation = Operation('testid', 'testapp', 'op1', target)
operation.run()
operation.join()
assert operation.thread_data['test_operation'] == operation
@ -164,7 +166,7 @@ def test_updating_operation(update_notification):
operation = Operation.get_operation()
operation.on_update('message1', exception)
operation = Operation('testapp', 'op1', target)
operation = Operation('testid', 'testapp', 'op1', target)
operation.run()
with pytest.raises(RuntimeError):
operation.join()
@ -177,7 +179,7 @@ def test_updating_operation(update_notification):
@patch('plinth.app.App.get')
def test_message(app_get):
"""Test getting the operation's message."""
operation = Operation('testapp', 'op1', Mock())
operation = Operation('testid', 'testapp', 'op1', Mock())
operation._message = 'message1'
operation.exception = RuntimeError('error1')
assert operation.message == 'message1'
@ -208,7 +210,8 @@ def test_message(app_get):
def test_update_notification(app_get):
"""Test that operation notification is created."""
app_get.return_value = AppTest()
operation = Operation('testapp', 'op1', Mock(), show_notification=True)
operation = Operation('testid', 'testapp', 'op1', Mock(),
show_notification=True)
note = Notification.get('testapp-operation')
assert note.id == 'testapp-operation'
assert note.app_id == 'testapp'
@ -239,7 +242,7 @@ def test_manager_global_instance():
def test_manager_init():
"""Test initializing operations manager."""
manager = OperationsManager()
assert manager._operations == []
assert manager._operations == {}
assert manager._current_operation is None
assert isinstance(manager._lock, threading.RLock().__class__)
@ -252,14 +255,15 @@ def test_manager_new():
def target():
event.wait()
operation = manager.new('testapp', 'op1', target)
operation = manager.new('testop', 'testapp', 'op1', target)
assert isinstance(operation, Operation)
assert manager._current_operation == operation
assert manager._operations == [operation]
assert manager._operations == {'testop': operation}
event.set()
operation.join()
assert manager._current_operation is None
assert manager._operations == [operation]
assert manager._operations == OrderedDict(testop=operation)
def test_manager_new_without_show_message():
@ -270,11 +274,28 @@ def test_manager_new_without_show_message():
def target():
event.wait()
operation = manager.new('testapp', 'op1', target, show_message=False)
operation = manager.new('testop', 'testapp', 'op1', target,
show_message=False)
event.set()
operation.join()
assert manager._current_operation is None
assert manager._operations == []
assert manager._operations == {}
def test_manager_new_raises():
"""Test that a new operation is always unique."""
manager = OperationsManager()
operation1 = manager.new('testop1', 'testapp', 'op1', Mock())
# Creating operation with same id throws exception
with pytest.raises(KeyError):
manager.new('testop1', 'testapp', 'op1', Mock())
# Creating operation with different ID works
operation2 = manager.new('testop2', 'testapp', 'op3', Mock())
assert manager._operations == OrderedDict(testop1=operation1,
testop2=operation2)
def test_manager_scheduling():
@ -284,13 +305,15 @@ def test_manager_scheduling():
event2 = threading.Event()
event3 = threading.Event()
operation1 = manager.new('testapp', 'op1', event1.wait)
operation2 = manager.new('testapp', 'op2', event2.wait)
operation3 = manager.new('testapp', 'op3', event3.wait)
operation1 = manager.new('testop1', 'testapp', 'op1', event1.wait)
operation2 = manager.new('testop2', 'testapp', 'op2', event2.wait)
operation3 = manager.new('testop3', 'testapp', 'op3', event3.wait)
def _assert_is_running(current_operation):
assert manager._current_operation == current_operation
assert manager._operations == [operation1, operation2, operation3]
assert manager._operations == OrderedDict(testop1=operation1,
testop2=operation2,
testop3=operation3)
for operation in [operation1, operation2, operation3]:
alive = (operation == current_operation)
assert operation.thread.is_alive() == alive
@ -311,9 +334,9 @@ def test_manager_scheduling():
def test_manager_filter():
"""Test returning filtered operations."""
manager = OperationsManager()
operation1 = manager.new('testapp1', 'op1', Mock())
operation2 = manager.new('testapp1', 'op2', Mock())
operation3 = manager.new('testapp2', 'op3', Mock())
operation1 = manager.new('testop1', 'testapp1', 'op1', Mock())
operation2 = manager.new('testop2', 'testapp1', 'op2', Mock())
operation3 = manager.new('testop3', 'testapp2', 'op3', Mock())
manager.filter('testapp1') == [operation1, operation2]
manager.filter('testapp2') == [operation3]
@ -323,14 +346,15 @@ def test_manager_collect_results():
manager = OperationsManager()
event = threading.Event()
operation1 = manager.new('testapp1', 'op1', Mock())
operation2 = manager.new('testapp2', 'op2', Mock())
operation3 = manager.new('testapp1', 'op3', event.wait)
operation1 = manager.new('testop1', 'testapp1', 'op1', Mock())
operation2 = manager.new('testop2', 'testapp2', 'op2', Mock())
operation3 = manager.new('testop3', 'testapp1', 'op3', event.wait)
operation1.join()
operation2.join()
assert manager.collect_results('testapp1') == [operation1]
assert manager._operations == [operation2, operation3]
assert manager._operations == OrderedDict(testop2=operation2,
testop3=operation3)
event.set()
operation3.join()
assert manager.collect_results('testapp1') == [operation3]
assert manager._operations == [operation2]
assert manager._operations == OrderedDict(testop2=operation2)