diff --git a/python/rpdk/python/codegen.py b/python/rpdk/python/codegen.py index f5c49cf3..b175b787 100644 --- a/python/rpdk/python/codegen.py +++ b/python/rpdk/python/codegen.py @@ -55,12 +55,16 @@ def __init__(self): self.package_name = None self.package_root = None self._use_docker = None + self._no_docker = None self._protocol_version = "2.0.0" def _init_from_project(self, project): self.namespace = tuple(s.lower() for s in project.type_info) self.package_name = "_".join(self.namespace) - self._use_docker = project.settings.get("use_docker") + # Check config file for (legacy) 'useDocker' and use_docker settings + self._use_docker = project.settings.get("useDocker") or project.settings.get( + "use_docker" + ) self.package_root = project.root / "src" def _init_settings(self, project): @@ -78,14 +82,29 @@ def _init_settings(self, project): ".resource", ".test_entrypoint" ) - self._use_docker = self._use_docker or input_with_validation( - "Use docker for platform-independent packaging (Y/n)?\n", - validate_no, - "This is highly recommended unless you are experienced \n" - "with cross-platform Python packaging.", - ) + # If use_docker specified in .rpdk-config file or cli switch + # Ensure only 1 is true, with preference to use_docker + if project.settings.get("use_docker") is True: + self._use_docker = True + self._no_docker = False + # If no_docker specified in .rpdk-config file or cli switch + elif project.settings.get("no_docker") is True: + self._use_docker = False + self._no_docker = True + else: + # If neither no_docker nor use_docker specified in .rpdk-config + # file or cli switch, prompt to use containers or not + self._use_docker = input_with_validation( + "Use docker for platform-independent packaging (Y/n)?\n", + validate_no, + "This is highly recommended unless you are experienced \n" + "with cross-platform Python packaging.", + ) + self._no_docker = not self._use_docker + # project.settings will get saved into .rpdk-config by cloudformation-cli project.settings["use_docker"] = self._use_docker + project.settings["no_docker"] = self._no_docker project.settings["protocolVersion"] = self._protocol_version def init(self, project): diff --git a/python/rpdk/python/parser.py b/python/rpdk/python/parser.py index 518dbcbf..04b55ff6 100644 --- a/python/rpdk/python/parser.py +++ b/python/rpdk/python/parser.py @@ -9,7 +9,9 @@ def setup_subparser(subparsers, parents, python_version, python_version_number): ) parser.set_defaults(language=python_version) - parser.add_argument( + group = parser.add_mutually_exclusive_group() + + group.add_argument( "-d", "--use-docker", action="store_true", @@ -18,6 +20,13 @@ def setup_subparser(subparsers, parents, python_version, python_version_number): with cross-platform Python packaging.""", ) + group.add_argument( + "--no-docker", + action="store_true", + help="""Generally not recommended unless you are experienced + with cross-platform Python packaging""", + ) + return parser diff --git a/tests/plugin/codegen_test.py b/tests/plugin/codegen_test.py index a36d04b4..d17b50d5 100644 --- a/tests/plugin/codegen_test.py +++ b/tests/plugin/codegen_test.py @@ -108,7 +108,32 @@ def resource_project(tmp_path): "rpdk.python.codegen.input_with_validation", autospec=True, side_effect=[False] ) with patch_plugins, patch_wizard: - project.init(TYPE_NAME, PythonLanguagePlugin.NAME) + project.init( + TYPE_NAME, + PythonLanguagePlugin.NAME, + settings={"use_docker": False, "no_docker": True}, + ) + return project + + +@pytest.fixture +def resource_project_use_docker(tmp_path): + project = Project(root=tmp_path) + + patch_plugins = patch.dict( + "rpdk.core.plugin_registry.PLUGIN_REGISTRY", + {PythonLanguagePlugin.NAME: lambda: PythonLanguagePlugin}, + clear=True, + ) + patch_wizard = patch( + "rpdk.python.codegen.input_with_validation", autospec=True, side_effect=[False] + ) + with patch_plugins, patch_wizard: + project.init( + TYPE_NAME, + PythonLanguagePlugin.NAME, + settings={"use_docker": True, "no_docker": False}, + ) return project @@ -125,7 +150,32 @@ def hook_project(tmp_path): "rpdk.python.codegen.input_with_validation", autospec=True, side_effect=[False] ) with patch_plugins, patch_wizard: - project.init_hook(TYPE_NAME, PythonLanguagePlugin.NAME) + project.init_hook( + TYPE_NAME, + PythonLanguagePlugin.NAME, + settings={"use_docker": False, "no_docker": True}, + ) + return project + + +@pytest.fixture +def hook_project_use_docker(tmp_path): + project = Project(root=tmp_path) + + patch_plugins = patch.dict( + "rpdk.core.plugin_registry.PLUGIN_REGISTRY", + {PythonLanguagePlugin.NAME: lambda: PythonLanguagePlugin}, + clear=True, + ) + patch_wizard = patch( + "rpdk.python.codegen.input_with_validation", autospec=True, side_effect=[False] + ) + with patch_plugins, patch_wizard: + project.init_hook( + TYPE_NAME, + PythonLanguagePlugin.NAME, + settings={"use_docker": True, "no_docker": False}, + ) return project @@ -172,6 +222,7 @@ def test__remove_build_artifacts_file_not_found(tmp_path): def test_initialize_resource(resource_project): assert resource_project.settings == { "use_docker": False, + "no_docker": True, "protocolVersion": "2.0.0", } @@ -209,8 +260,53 @@ def test_initialize_resource(resource_project): ast.parse(files[f"{os.path.join('src', 'foo_bar_baz', 'handlers.py')}"].read_text()) +def test_initialize_resource_use_docker(resource_project_use_docker): + assert resource_project_use_docker.settings == { + "use_docker": True, + "no_docker": False, + "protocolVersion": "2.0.0", + } + + files = get_files_in_project(resource_project_use_docker) + assert set(files) == { + ".gitignore", + ".rpdk-config", + "README.md", + "foo-bar-baz.json", + "requirements.txt", + f"{os.path.join('example_inputs', 'inputs_1_create.json')}", + f"{os.path.join('example_inputs', 'inputs_1_invalid.json')}", + f"{os.path.join('example_inputs', 'inputs_1_update.json')}", + "example_inputs", + "src", + f"{os.path.join('src', 'foo_bar_baz')}", + f"{os.path.join('src', 'foo_bar_baz', '__init__.py')}", + f"{os.path.join('src', 'foo_bar_baz', 'handlers.py')}", + "template.yml", + } + + assert "__pycache__" in files[".gitignore"].read_text() + assert SUPPORT_LIB_NAME in files["requirements.txt"].read_text() + + readme = files["README.md"].read_text() + assert resource_project_use_docker.type_name in readme + assert SUPPORT_LIB_PKG in readme + assert "handlers.py" in readme + assert "models.py" in readme + + assert resource_project_use_docker.entrypoint in files["template.yml"].read_text() + + # this is a rough check the generated Python code is valid as far as syntax + ast.parse(files[f"{os.path.join('src', 'foo_bar_baz', '__init__.py')}"].read_text()) + ast.parse(files[f"{os.path.join('src', 'foo_bar_baz', 'handlers.py')}"].read_text()) + + def test_initialize_hook(hook_project): - assert hook_project.settings == {"use_docker": False, "protocolVersion": "2.0.0"} + assert hook_project.settings == { + "use_docker": False, + "no_docker": True, + "protocolVersion": "2.0.0", + } files = get_files_in_project(hook_project) assert set(files) == { @@ -242,6 +338,43 @@ def test_initialize_hook(hook_project): ast.parse(files[f"{os.path.join('src', 'foo_bar_baz', 'handlers.py')}"].read_text()) +def test_initialize_hook_use_docker(hook_project_use_docker): + assert hook_project_use_docker.settings == { + "use_docker": True, + "no_docker": False, + "protocolVersion": "2.0.0", + } + + files = get_files_in_project(hook_project_use_docker) + assert set(files) == { + ".gitignore", + ".rpdk-config", + "README.md", + "foo-bar-baz.json", + "requirements.txt", + "src", + f"{os.path.join('src', 'foo_bar_baz')}", + f"{os.path.join('src', 'foo_bar_baz', '__init__.py')}", + f"{os.path.join('src', 'foo_bar_baz', 'handlers.py')}", + "template.yml", + } + + assert "__pycache__" in files[".gitignore"].read_text() + assert SUPPORT_LIB_NAME in files["requirements.txt"].read_text() + + readme = files["README.md"].read_text() + assert hook_project_use_docker.type_name in readme + assert SUPPORT_LIB_PKG in readme + assert "handlers.py" in readme + assert "models.py" in readme + + assert hook_project_use_docker.entrypoint in files["template.yml"].read_text() + + # this is a rough check the generated Python code is valid as far as syntax + ast.parse(files[f"{os.path.join('src', 'foo_bar_baz', '__init__.py')}"].read_text()) + ast.parse(files[f"{os.path.join('src', 'foo_bar_baz', 'handlers.py')}"].read_text()) + + def test_generate_resource(resource_project): resource_project.load_schema() before = get_files_in_project(resource_project) @@ -406,6 +539,7 @@ def test__pip_build_called_process_error(tmp_path): def test__build_pip(plugin): plugin._use_docker = False + plugin._no_docker = True patch_pip = patch.object(plugin, "_pip_build", autospec=True) patch_docker = patch.object(plugin, "_docker_build", autospec=True) @@ -418,6 +552,7 @@ def test__build_pip(plugin): def test__build_docker(plugin): plugin._use_docker = True + plugin._no_docker = False patch_pip = patch.object(plugin, "_pip_build", autospec=True) patch_docker = patch.object(plugin, "_docker_build", autospec=True) @@ -431,6 +566,7 @@ def test__build_docker(plugin): # Test _build_docker on Linux/Unix-like systems def test__build_docker_posix(plugin): plugin._use_docker = True + plugin._no_docker = False patch_pip = patch.object(plugin, "_pip_build", autospec=True) patch_from_env = patch("rpdk.python.codegen.docker.from_env", autospec=True) @@ -456,6 +592,7 @@ def test__build_docker_posix(plugin): # Test _build_docker on Windows def test__build_docker_windows(plugin): plugin._use_docker = True + plugin._no_docker = False patch_pip = patch.object(plugin, "_pip_build", autospec=True) patch_from_env = patch("rpdk.python.codegen.docker.from_env", autospec=True) @@ -481,6 +618,7 @@ def test__build_docker_windows(plugin): # Test _build_docker if geteuid fails def test__build_docker_no_euid(plugin): plugin._use_docker = True + plugin._no_docker = False patch_pip = patch.object(plugin, "_pip_build", autospec=True) patch_from_env = patch("rpdk.python.codegen.docker.from_env", autospec=True) diff --git a/tests/plugin/parser_test.py b/tests/plugin/parser_test.py index cc180145..f29cdbf8 100644 --- a/tests/plugin/parser_test.py +++ b/tests/plugin/parser_test.py @@ -1,3 +1,5 @@ +import pytest + import argparse from rpdk.python.parser import ( setup_subparser_python36, @@ -13,10 +15,29 @@ def test_setup_subparser_python36(): sub_parser = setup_subparser_python36(subparsers, []) - args = sub_parser.parse_args(["-d"]) + args = sub_parser.parse_args([]) assert args.language == "python36" - assert args.use_docker is True + assert args.use_docker is False + assert args.no_docker is False + + short_args = sub_parser.parse_args(["-d"]) + assert short_args.language == "python36" + assert short_args.use_docker is True + assert short_args.no_docker is False + + long_args = sub_parser.parse_args(["--use-docker"]) + assert long_args.language == "python36" + assert long_args.use_docker is True + assert long_args.no_docker is False + + no_docker = sub_parser.parse_args(["--no-docker"]) + assert no_docker.language == "python36" + assert no_docker.use_docker is False + assert no_docker.no_docker is True + + with pytest.raises(SystemExit): + sub_parser.parse_args(["--no-docker", "--use-docker"]) def test_setup_subparser_python37(): @@ -29,6 +50,25 @@ def test_setup_subparser_python37(): assert args.language == "python37" assert args.use_docker is False + assert args.no_docker is False + + short_args = sub_parser.parse_args(["-d"]) + assert short_args.language == "python37" + assert short_args.use_docker is True + assert short_args.no_docker is False + + long_args = sub_parser.parse_args(["--use-docker"]) + assert long_args.language == "python37" + assert long_args.use_docker is True + assert long_args.no_docker is False + + no_docker = sub_parser.parse_args(["--no-docker"]) + assert no_docker.language == "python37" + assert no_docker.use_docker is False + assert no_docker.no_docker is True + + with pytest.raises(SystemExit): + sub_parser.parse_args(["--no-docker", "--use-docker"]) def test_setup_subparser_python38(): @@ -41,6 +81,25 @@ def test_setup_subparser_python38(): assert args.language == "python38" assert args.use_docker is False + assert args.no_docker is False + + short_args = sub_parser.parse_args(["-d"]) + assert short_args.language == "python38" + assert short_args.use_docker is True + assert short_args.no_docker is False + + long_args = sub_parser.parse_args(["--use-docker"]) + assert long_args.language == "python38" + assert long_args.use_docker is True + assert long_args.no_docker is False + + no_docker = sub_parser.parse_args(["--no-docker"]) + assert no_docker.language == "python38" + assert no_docker.use_docker is False + assert no_docker.no_docker is True + + with pytest.raises(SystemExit): + sub_parser.parse_args(["--no-docker", "--use-docker"]) def test_setup_subparser_python39(): @@ -53,3 +112,22 @@ def test_setup_subparser_python39(): assert args.language == "python39" assert args.use_docker is False + assert args.no_docker is False + + short_args = sub_parser.parse_args(["-d"]) + assert short_args.language == "python39" + assert short_args.use_docker is True + assert short_args.no_docker is False + + long_args = sub_parser.parse_args(["--use-docker"]) + assert long_args.language == "python39" + assert long_args.use_docker is True + assert long_args.no_docker is False + + no_docker = sub_parser.parse_args(["--no-docker"]) + assert no_docker.language == "python39" + assert no_docker.use_docker is False + assert no_docker.no_docker is True + + with pytest.raises(SystemExit): + sub_parser.parse_args(["--no-docker", "--use-docker"])