From 5e6c6767488fc49783d7cc3d21539bd762c0776f Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Wed, 1 Nov 2023 11:32:24 -0700 Subject: [PATCH] operation: Fix issue with re-running setup when it fails first time When setup is run from the application thread after startup, it continuously tries until it succeeds. However, after making the first attempt, it does not collect the status of the operation keeping the operation object in operation manager. When trying for the second time, trying to create operation with same ID fails since the operation is already present. Fix this by allowing the operation to be recreated if the existing operation has failed. Tests: - Unit tests pass. - Functional tests for bepasty app pass. - Install an app. Create an error in the setup mechanism for an app. Increment is app version number. Start the service and notice that setup of app is attempted and fails. Few seconds later the setup is attempted again and the process continues. Each time the failure is due to fault in the app's setup method rather than operation not being accepted. Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- plinth/operation.py | 3 ++- plinth/tests/test_operation.py | 23 ++++++++++++++++++----- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/plinth/operation.py b/plinth/operation.py index acbcbe016..ce8c3095c 100644 --- a/plinth/operation.py +++ b/plinth/operation.py @@ -187,7 +187,8 @@ class OperationsManager: 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: + if (op_id in self._operations and self._operations[op_id].state != + Operation.State.COMPLETED): raise KeyError('Operation in progress/scheduled') kwargs['on_complete'] = self._on_operation_complete diff --git a/plinth/tests/test_operation.py b/plinth/tests/test_operation.py index f4baaeeb1..d3bd45ce9 100644 --- a/plinth/tests/test_operation.py +++ b/plinth/tests/test_operation.py @@ -285,16 +285,29 @@ def test_manager_new_without_show_message(): def test_manager_new_raises(): """Test that a new operation is always unique.""" manager = OperationsManager() - operation1 = manager.new('testop1', 'testapp', 'op1', Mock()) + event1 = threading.Event() + event2 = threading.Event() - # Creating operation with same id throws exception + operation1 = manager.new('testop1', 'testapp', 'op1', + lambda: event1.wait()) + + # Creating operation with different ID works + operation2 = manager.new('testop2', 'testapp', 'op3', lambda: event2.set()) + + assert manager._operations == OrderedDict(testop1=operation1, + testop2=operation2) + + # Creating operation with same id while the operation is running/scheduled + # 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()) + # Creating operation with same id after the operation is completed works. + event1.set() # Allow the first operation to complete + event2.wait(10) # Wait until the second operation has started + operation1_new = manager.new('testop1', 'testapp', 'op1', Mock()) - assert manager._operations == OrderedDict(testop1=operation1, + assert manager._operations == OrderedDict(testop1=operation1_new, testop2=operation2)