torch/runner: rename stop command to cancel (#501)

Summary:
Pull Request resolved: #501

See #497
* added cancel command to be consistent with CLI and scheduler API
* updated CLI cancel command to call runner.cancel()
* updated unit tests
* added warning message when using runner.stop()

Reviewed By: d4l3k

Differential Revision: D36620537

fbshipit-source-id: 8bb5415d3021d21a91cbd21458126e030bc162c2

Author: chrisjwiebe

torchx/cli/cmd_cancel.py

@@ -27,4 +27,4 @@ def run(self, args: argparse.Namespace) -> None:
         app_handle = args.app_handle
         _, session_name, _ = api.parse_app_handle(app_handle)
         runner = get_runner(name=session_name)
-        runner.stop(app_handle)
+        runner.cancel(app_handle)

torchx/cli/test/cmd_cancel_test.py

@@ -13,14 +13,14 @@
 
 
 class CmdCancelTest(unittest.TestCase):
-    @patch("torchx.runner.api.Runner.stop")
-    def test_run(self, stop: MagicMock) -> None:
+    @patch("torchx.runner.api.Runner.cancel")
+    def test_run(self, cancel: MagicMock) -> None:
         parser = argparse.ArgumentParser()
         cmd_runopts = CmdCancel()
         cmd_runopts.add_arguments(parser)
 
         args = parser.parse_args(["foo://session/id"])
         cmd_runopts.run(args)
 
-        self.assertEqual(stop.call_count, 1)
-        stop.assert_called_with("foo://session/id")
+        self.assertEqual(cancel.call_count, 1)
+        cancel.assert_called_with("foo://session/id")

torchx/runner/api.py

@@ -405,7 +405,7 @@ def list(self) -> Dict[AppHandle, AppDef]:
                 self.status(app_id)
             return self._apps
 
-    def stop(self, app_handle: AppHandle) -> None:
+    def cancel(self, app_handle: AppHandle) -> None:
         """
         Stops the application, effectively directing the scheduler to cancel
         the job. Does nothing if the app does not exist.
@@ -419,11 +419,25 @@ def stop(self, app_handle: AppHandle) -> None:
 
         """
         scheduler, scheduler_backend, app_id = self._scheduler_app_id(app_handle)
-        with log_event("stop", scheduler_backend, app_id):
+        with log_event("cancel", scheduler_backend, app_id):
             status = self.status(app_handle)
             if status is not None and not status.is_terminal():
                 scheduler.cancel(app_id)
 
+    def stop(self, app_handle: AppHandle) -> None:
+        """
+        See method ``cancel``.
+
+        .. warning:: This method will be deprecated in the future. It has been
+                    replaced with ``cancel`` which provides the same functionality.
+                    The change is to be consistent with the CLI and scheduler API.
+        """
+        warnings.warn(
+            "This method will be deprecated in the future, please use `cancel` instead.",
+            PendingDeprecationWarning,
+        )
+        self.cancel(app_handle)
+
     def describe(self, app_handle: AppHandle) -> Optional[AppDef]:
         """
         Reconstructs the application (to the best extent) given the app handle.

torchx/runner/test/api_test.py

@@ -293,7 +293,7 @@ def test_status(self, _) -> None:
             app_handle = runner.run(app, scheduler="local_dir", cfg=self.cfg)
             app_status = none_throws(runner.status(app_handle))
             self.assertEqual(AppState.RUNNING, app_status.state)
-            runner.stop(app_handle)
+            runner.cancel(app_handle)
             app_status = none_throws(runner.status(app_handle))
             self.assertEqual(AppState.CANCELLED, app_status.state)
 
@@ -364,6 +364,13 @@ def test_wait_unknown_app(self, _) -> None:
                 runner.wait("local_dir://another_session/some_app", wait_interval=0.1)
             )
 
+    def test_cancel(self, _) -> None:
+        with Runner(
+            name=SESSION_NAME,
+            schedulers={"local_dir": self.scheduler},
+        ) as runner:
+            self.assertIsNone(runner.cancel("local_dir://test_session/unknown_app_id"))
+
     def test_stop(self, _) -> None:
         with Runner(
             name=SESSION_NAME,