From e56918bf4796c821d240eb9314758c77690a6566 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Behmo?= Date: Tue, 18 Oct 2022 14:40:32 +0200 Subject: [PATCH] depr: get rid of the `local/dev bindmount` commands This command has always been clunky. It is now removed in favour of the `-m/--mount` option. Close https://github.com/overhangio/2u-tutor-adoption/issues/88 Close https://github.com/overhangio/2u-tutor-adoption/issues/89 --- CHANGELOG-nightly.md | 1 + docs/dev.rst | 18 ------------------ tests/commands/test_dev.py | 5 ----- tests/test_bindmounts.py | 36 ----------------------------------- tutor/bindmounts.py | 22 --------------------- tutor/commands/compose.py | 39 +------------------------------------- 6 files changed, 2 insertions(+), 119 deletions(-) delete mode 100644 tests/test_bindmounts.py diff --git a/CHANGELOG-nightly.md b/CHANGELOG-nightly.md index 5f89eb7..ee725c9 100644 --- a/CHANGELOG-nightly.md +++ b/CHANGELOG-nightly.md @@ -8,6 +8,7 @@ will be backported to the master branch at every major release. When backporting changes to master, we should keep only the entries that correspond to user- facing changes. --> +- 💥[Improvement] Remove the `local/dev bindmount` commands, which have been marked as deprecated for some time. The `--mount` option should be used instead. - 💥[Bugfix] Fix local installation requirements. Plugins that implemented the "openedx-dockerfile-post-python-requirements" patch and that needed access to the edx-platform repo will no longer work. Instead, these plugins should implement the "openedx-dockerfile-pre-assets" patch. This scenario should be very rare, though. (by @regisb) - 💥[Improvement] Rename the implementation of tutor quickstart to tutor launch. (by @Carlos-Muniz) - 💥[Improvement] Remove the implementation of tutor dev runserver. (by @Carlos-Muniz) diff --git a/docs/dev.rst b/docs/dev.rst index d39eefc..d52457f 100644 --- a/docs/dev.rst +++ b/docs/dev.rst @@ -152,24 +152,6 @@ Then, bind-mount that folder back in the container with the ``--mount`` option ( You can then edit the files in ``~/venv`` on your local filesystem and see the changes live in your container. -Bind-mount from the "volumes/" directory -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -.. warning:: Bind-mounting volumes with the ``bindmount`` command is no longer the default, recommended way of bind-mounting volumes from the host. Instead, see the :ref:`mount option ` and the ``tutor dev/local copyfrom`` commands. - -Tutor makes it easy to create a bind-mount from an existing container. First, copy the contents of a container directory with the ``bindmount`` command. For instance, to copy the virtual environment of the "lms" container:: - - tutor dev bindmount lms /openedx/venv - -This command recursively copies the contents of the ``/opendedx/venv`` directory to ``$(tutor config printroot)/volumes/venv``. The code of any Python dependency can then be edited -- for instance, you can then add a ``breakpoint()`` statement for step-by-step debugging, or implement a custom feature. - -Then, bind-mount the directory back in the container with the ``--mount`` option:: - - tutor dev start --mount=lms:$(tutor config printroot)/volumes/venv:/openedx/venv lms - -.. note:: - The ``bindmount`` command and the ``--mount=...`` option syntax are available both for the ``tutor local`` and ``tutor dev`` commands. - Manual bind-mount to any directory ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/tests/commands/test_dev.py b/tests/commands/test_dev.py index 2f911f5..0b962d7 100644 --- a/tests/commands/test_dev.py +++ b/tests/commands/test_dev.py @@ -8,8 +8,3 @@ class DevTests(unittest.TestCase, TestCommandMixin): result = self.invoke(["dev", "--help"]) self.assertEqual(0, result.exit_code) self.assertIsNone(result.exception) - - def test_dev_bindmount(self) -> None: - result = self.invoke(["dev", "bindmount", "--help"]) - self.assertEqual(0, result.exit_code) - self.assertIsNone(result.exception) diff --git a/tests/test_bindmounts.py b/tests/test_bindmounts.py deleted file mode 100644 index dbc4b4b..0000000 --- a/tests/test_bindmounts.py +++ /dev/null @@ -1,36 +0,0 @@ -import unittest - -from tutor import bindmounts -from tutor.exceptions import TutorError - - -class BindMountsTests(unittest.TestCase): - def test_get_name(self) -> None: - self.assertEqual("venv", bindmounts.get_name("/openedx/venv")) - self.assertEqual("venv", bindmounts.get_name("/openedx/venv/")) - - def test_get_name_root_folder(self) -> None: - with self.assertRaises(TutorError): - bindmounts.get_name("/") - with self.assertRaises(TutorError): - bindmounts.get_name("") - - def test_parse_volumes(self) -> None: - volume_args, non_volume_args = bindmounts.parse_volumes( - [ - "run", - "--volume=/openedx/venv", - "-v", - "/tmp/openedx:/openedx", - "lms", - "echo", - "boom", - ] - ) - self.assertEqual(("/openedx/venv", "/tmp/openedx:/openedx"), volume_args) - self.assertEqual(("run", "lms", "echo", "boom"), non_volume_args) - - def test_parse_volumes_empty_list(self) -> None: - volume_args, non_volume_args = bindmounts.parse_volumes([]) - self.assertEqual((), volume_args) - self.assertEqual((), non_volume_args) diff --git a/tutor/bindmounts.py b/tutor/bindmounts.py index 874507d..4bc156e 100644 --- a/tutor/bindmounts.py +++ b/tutor/bindmounts.py @@ -1,7 +1,4 @@ import os -from typing import List, Tuple - -import click from .exceptions import TutorError from .jobs import BaseComposeJobRunner @@ -62,22 +59,3 @@ def get_name(container_bind_path: str) -> str: def get_root_path(root: str) -> str: return os.path.join(root, "volumes") - - -def parse_volumes(docker_compose_args: List[str]) -> Tuple[List[str], List[str]]: - """ - Parse `-v/--volume` options from an arbitrary list of arguments. - """ - - @click.command(context_settings={"ignore_unknown_options": True}) - @click.option("-v", "--volume", "volumes", multiple=True) - @click.argument("args", nargs=-1) - def custom_docker_compose( - volumes: List[str], args: List[str] # pylint: disable=unused-argument - ) -> None: - pass - - if isinstance(docker_compose_args, tuple): - docker_compose_args = list(docker_compose_args) - context = custom_docker_compose.make_context("custom", docker_compose_args) - return context.params["volumes"], context.params["args"] diff --git a/tutor/commands/compose.py b/tutor/commands/compose.py index 31d1910..6b62b6f 100644 --- a/tutor/commands/compose.py +++ b/tutor/commands/compose.py @@ -5,7 +5,6 @@ from copy import deepcopy import click -from tutor import bindmounts from tutor import config as tutor_config from tutor import env as tutor_env from tutor import fmt, hooks, jobs, serialize, utils @@ -392,28 +391,6 @@ def run( context.invoke(dc_command, mounts=mounts, command="run", args=[*extra_args, *args]) -@click.command( - name="bindmount", - help="Copy the contents of a container directory to a ready-to-bind-mount host directory", -) -@click.argument("service") -@click.argument("path") -@click.pass_obj -def bindmount_command(context: BaseComposeContext, service: str, path: str) -> None: - """ - This command is made obsolete by the --mount arguments. - """ - fmt.echo_alert( - "The 'bindmount' command is deprecated and will be removed in a later release. Use 'copyfrom' instead." - ) - config = tutor_config.load(context.root) - host_path = bindmounts.create(context.job_runner(config), service, path) - fmt.echo_info( - f"Bind-mount volume created at {host_path}. You can now use it in all `local` and `dev` " - f"commands with the `--volume={path}` option." - ) - - @click.command( name="copyfrom", help="Copy files/folders from a container directory to the local filesystem.", @@ -520,20 +497,7 @@ def dc_command( ) -> None: mount_tmp_volumes(mounts, context) config = tutor_config.load(context.root) - volumes, non_volume_args = bindmounts.parse_volumes(args) - volume_args = [] - for volume_arg in volumes: - if ":" not in volume_arg: - # This is a bind-mounted volume from the "volumes/" folder. - host_bind_path = bindmounts.get_path(context.root, volume_arg) - if not os.path.exists(host_bind_path): - raise TutorError( - f"Bind-mount volume directory {host_bind_path} does not exist. It must first be created " - f"with the '{bindmount_command.name}' command." - ) - volume_arg = f"{host_bind_path}:{volume_arg}" - volume_args += ["--volume", volume_arg] - context.job_runner(config).docker_compose(command, *volume_args, *non_volume_args) + context.job_runner(config).docker_compose(command, *args) @hooks.Filters.COMPOSE_MOUNTS.add() @@ -569,7 +533,6 @@ def add_commands(command_group: click.Group) -> None: command_group.add_command(dc_command) command_group.add_command(run) command_group.add_command(copyfrom) - command_group.add_command(bindmount_command) command_group.add_command(execute) command_group.add_command(logs) command_group.add_command(status)