Change cancel method to never raise (#420)

* Change cancel method to never raise

* Fix missing reference in Sphinx docs

* Fix missing word in changelog entry

* Clarify the 'cancel' change

* Reword description of cancel

docs/source/changes.rst

@@ -39,6 +39,13 @@ of Traits Futures.
   background task types.
 * The ``state`` trait of the ``~.TraitsExecutor`` is now read-only;
   previously, it was writable.
+* The ``cancel`` method of a future no longer raises :exc:`RuntimeError` when a
+  future is not cancellable. Instead, it communicates the information via its
+  return value. If a future is already done, or has previously been cancelled,
+  calling ``cancel`` on that future does not change the state of the future,
+  and returns ``False``. Otherwise it changes the future's state to
+  ``CANCELLING`` state, requests cancellation of the associated task, and
+  returns ``True``.
 * The ``ITaskSpecification.background_task`` method has been renamed to
   ``task``.
 * The ``ITaskSpecification.future`` method now requires a cancellation callback

docs/source/guide/examples/background_processes.py

@@ -133,8 +133,7 @@ def _square_fired(self):
 
     def _cancel_all_fired(self):
         for future in self.current_futures:
-            if future.cancellable:
-                future.cancel()
+            future.cancel()
 
     def _clear_finished_fired(self):
         for future in list(self.current_futures):

docs/source/guide/examples/slow_squares.py

@@ -121,8 +121,7 @@ def _square_fired(self):
 
     def _cancel_all_fired(self):
         for future in self.current_futures:
-            if future.cancellable:
-                future.cancel()
+            future.cancel()
 
     def _clear_finished_fired(self):
         for future in list(self.current_futures):

docs/source/guide/intro.rst

@@ -241,14 +241,19 @@ For |ProgressFuture|, the |cancel| method causes a running
 task to abort the next time that task calls ``progress``. No further
 progress results are received after calling |cancel|.
 
-In all cases, a future may only be cancelled if its state is one of |WAITING|
-or |EXECUTING|. Attempting to cancel a future in another state will raise a
-|RuntimeError|. Calling |cancel| immediately puts the future into
-|CANCELLING| state, and the state is updated to |CANCELLED| once the future has
-finished executing. No results or exception information are received from a
-future in |CANCELLING| state. A cancelled future will never reach |FAILED|
-state, and will never record information from a background task exception that
-occurs after the |cancel| call.
+In all cases, a task may only be cancelled if the state of the associated
+future is either |WAITING| or |EXECUTING|. When |cancel| is called on a future
+in one of these two states, the future's state is changed to |CANCELLING|,
+a cancellation request is sent to the associated task, and the call returns
+``True``. When |cancel| is called on a future in another state, the call has
+no effect, and returns ``False``.
+
+A successful |cancel| immediately puts the future into |CANCELLING| state, and
+the state is updated to |CANCELLED| once the future has finished executing. No
+results or exception information are received from a future in |CANCELLING|
+state. A cancelled future will never reach |FAILED| state, and will never
+record information from a background task exception that occurs after the
+|cancel| call.
 
 
 Stopping the executor

traits_futures/base_future.py

@@ -12,6 +12,8 @@
 Base class providing common pieces of the Future machinery.
 """
 
+import logging
+
 from traits.api import (
     Any,
     Bool,
@@ -39,6 +41,8 @@
 )
 from traits_futures.i_future import IFuture
 
+logger = logging.getLogger(__name__)
+
 # Messages sent by the BaseTask, and interpreted by BaseFuture.
 
 #: Custom message from the future. The argument is a pair
@@ -221,20 +225,26 @@ def cancel(self):
 
         A task in ``WAITING`` or ``EXECUTING`` state will immediately be moved
         to ``CANCELLING`` state. If the task is not in ``WAITING`` or
-        ``EXECUTING`` state, this function will raise ``RuntimeError``.
+        ``EXECUTING`` state, this function does nothing.
 
-        Raises
-        ------
-        RuntimeError
-            If the task has already completed or cancellation has already
-            been requested.
+        .. versionchanged:: 0.3.0
+
+           This method no longer raises for a task that isn't cancellable.
+           In previous versions, :exc:`RuntimeError` was raised.
+
+        Returns
+        -------
+        cancelled : bool
+            True if the task was cancelled, False if the task was not
+            cancellable.
         """
-        if not self.cancellable:
-            raise RuntimeError(
-                "Can only cancel a waiting or executing task. "
-                "Task state is {}".format(self.state)
-            )
-        self._user_cancelled()
+        if self.cancellable:
+            self._user_cancelled()
+            logger.debug(f"{self} cancelled")
+            return True
+        else:
+            logger.debug(f"{self} not cancellable; state is {self.state}")
+            return False
 
     def receive(self, message):
         """

traits_futures/i_future.py

@@ -97,13 +97,18 @@ def cancel(self):
 
         A task in ``WAITING`` or ``EXECUTING`` state will immediately be moved
         to ``CANCELLING`` state. If the task is not in ``WAITING`` or
-        ``EXECUTING`` state, this function will raise ``RuntimeError``.
+        ``EXECUTING`` state, this function does nothing.
 
-        Raises
-        ------
-        RuntimeError
-            If the task has already completed or cancellation has already
-            been requested.
+        .. versionchanged:: 0.3.0
+
+           This method no longer raises for a task that isn't cancellable.
+           In previous versions, :exc:`RuntimeError` was raised.
+
+        Returns
+        -------
+        cancelled : bool
+            True if the task was cancelled, False if the task was not
+            cancellable.
         """
 
     @abc.abstractmethod

traits_futures/tests/background_call_tests.py

@@ -191,8 +191,8 @@ def test_cannot_cancel_after_success(self):
         self.wait_until_done(future)
 
         self.assertFalse(future.cancellable)
-        with self.assertRaises(RuntimeError):
-            future.cancel()
+        cancelled = future.cancel()
+        self.assertFalse(cancelled)
 
         self.assertResult(future, 8)
         self.assertNoException(future)
@@ -208,8 +208,8 @@ def test_cannot_cancel_after_failure(self):
         self.wait_until_done(future)
 
         self.assertFalse(future.cancellable)
-        with self.assertRaises(RuntimeError):
-            future.cancel()
+        cancelled = future.cancel()
+        self.assertFalse(cancelled)
 
         self.assertNoResult(future)
         self.assertException(future, ZeroDivisionError)
@@ -223,10 +223,12 @@ def test_cannot_cancel_after_cancel(self):
         listener = CallFutureListener(future=future)
 
         self.assertTrue(future.cancellable)
-        future.cancel()
+        cancelled = future.cancel()
+        self.assertTrue(cancelled)
+        self.assertFalse(future.cancellable)
+        cancelled = future.cancel()
+        self.assertFalse(cancelled)
         self.assertFalse(future.cancellable)
-        with self.assertRaises(RuntimeError):
-            future.cancel()
 
         self.wait_until_done(future)
 
@@ -254,8 +256,8 @@ def test_double_cancel_variant(self):
         test_ready.set()
 
         self.assertFalse(future.cancellable)
-        with self.assertRaises(RuntimeError):
-            future.cancel()
+        cancelled = future.cancel()
+        self.assertFalse(cancelled)
 
         self.wait_until_done(future)
 

traits_futures/tests/background_iteration_tests.py

@@ -337,17 +337,17 @@ def test_double_cancel(self):
         future.cancel()
         self.assertFalse(future.cancellable)
 
-        with self.assertRaises(RuntimeError):
-            future.cancel()
+        cancelled = future.cancel()
+        self.assertFalse(cancelled)
 
     def test_completed_cancel(self):
         future = submit_iteration(self.executor, squares, 0, 10)
 
         self.wait_until_done(future)
 
         self.assertFalse(future.cancellable)
-        with self.assertRaises(RuntimeError):
-            future.cancel()
+        cancelled = future.cancel()
+        self.assertFalse(cancelled)
 
     def test_generator_closed_on_cancellation(self):
         resource_acquired = self._context.event()

traits_futures/tests/background_progress_tests.py

@@ -247,8 +247,8 @@ def test_double_cancellation(self):
         future.cancel()
 
         self.assertFalse(future.cancellable)
-        with self.assertRaises(RuntimeError):
-            future.cancel()
+        cancelled = future.cancel()
+        self.assertFalse(cancelled)
 
     def test_cancel_raising_task(self):
         signal = self._context.event()

traits_futures/traits_executor.py

@@ -469,10 +469,7 @@ def _cancel_tasks(self):
         logger.debug(f"{self} cancelling incomplete tasks")
         cancel_count = 0
         for wrapper in self._wrappers:
-            future = wrapper.future
-            if future.cancellable:
-                future.cancel()
-                cancel_count += 1
+            cancel_count += wrapper.future.cancel()
         logger.debug(f"{self} cancelled {cancel_count} tasks")
 
     def _stop_router(self):