From 5b4dc378574ec32633c19e44150ff86366493308 Mon Sep 17 00:00:00 2001 From: Dylan Anthony Date: Sun, 5 Sep 2021 12:49:56 -0600 Subject: [PATCH 1/2] fix: Don't crash the generator when one of the post-generation hooks is missing [fixes #479]. Thanks @chamini2 and @karolzlot! --- openapi_python_client/__init__.py | 61 +++++++++++--------- openapi_python_client/config.py | 7 ++- tests/test___init__.py | 95 +++++++++++++++++-------------- 3 files changed, 93 insertions(+), 70 deletions(-) diff --git a/openapi_python_client/__init__.py b/openapi_python_client/__init__.py index ef67b7013..b34841cdc 100644 --- a/openapi_python_client/__init__.py +++ b/openapi_python_client/__init__.py @@ -5,7 +5,8 @@ import sys from enum import Enum from pathlib import Path -from typing import Any, Dict, Optional, Sequence, Union +from subprocess import CalledProcessError +from typing import Any, Dict, List, Optional, Sequence, Union import httpcore import httpx @@ -16,7 +17,7 @@ from .config import Config from .parser import GeneratorData, import_string_from_class -from .parser.errors import GeneratorError +from .parser.errors import ErrorLevel, GeneratorError if sys.version_info.minor < 8: # version did not exist before 3.8, need to use a backport from importlib_metadata import version @@ -96,6 +97,7 @@ def __init__( project_name=self.project_name, project_dir=self.project_dir, ) + self.errors: List[GeneratorError] = [] def build(self) -> Sequence[GeneratorError]: """Create the project from templates""" @@ -112,7 +114,7 @@ def build(self) -> Sequence[GeneratorError]: self._build_metadata() self._build_models() self._build_api() - self._reformat() + self._run_post_hooks() return self._get_errors() def update(self) -> Sequence[GeneratorError]: @@ -125,35 +127,42 @@ def update(self) -> Sequence[GeneratorError]: self._create_package() self._build_models() self._build_api() - self._reformat() + self._run_post_hooks() return self._get_errors() - def _reformat(self) -> None: - subprocess.run( - "autoflake -i -r --remove-all-unused-imports --remove-unused-variables --ignore-init-module-imports .", - cwd=self.package_dir, - shell=True, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - check=True, - ) - subprocess.run( - "isort .", - cwd=self.project_dir, - shell=True, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - check=True, - ) - subprocess.run( - "black .", cwd=self.project_dir, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, check=True - ) + def _run_post_hooks(self) -> None: + for command in self.config.post_hooks: + self._run_command(command) + + def _run_command(self, cmd: str) -> None: + cmd_name = cmd.split(" ")[0] + command_exists = shutil.which(cmd_name) + if not command_exists: + self.errors.append( + GeneratorError( + level=ErrorLevel.WARNING, header="Skipping Integration", detail=f"{cmd_name} is not in PATH" + ) + ) + return + try: + subprocess.run( + cmd, cwd=self.project_dir, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, check=True + ) + except CalledProcessError as err: + self.errors.append( + GeneratorError( + level=ErrorLevel.ERROR, + header=f"{cmd_name} failed", + detail=err.stderr.decode() or err.output.decode(), + ) + ) - def _get_errors(self) -> Sequence[GeneratorError]: - errors = [] + def _get_errors(self) -> List[GeneratorError]: + errors: List[GeneratorError] = [] for collection in self.openapi.endpoint_collections_by_tag.values(): errors.extend(collection.parse_errors) errors.extend(self.openapi.errors) + errors.extend(self.errors) return errors def _create_package(self) -> None: diff --git a/openapi_python_client/config.py b/openapi_python_client/config.py index d9bd9e18e..2597d1aed 100644 --- a/openapi_python_client/config.py +++ b/openapi_python_client/config.py @@ -1,5 +1,5 @@ from pathlib import Path -from typing import Dict, Optional +from typing import Dict, List, Optional import yaml from pydantic import BaseModel @@ -25,6 +25,11 @@ class Config(BaseModel): project_name_override: Optional[str] package_name_override: Optional[str] package_version_override: Optional[str] + post_hooks: List[str] = [ + "autoflake -i -r --remove-all-unused-imports --remove-unused-variables --ignore-init-module-imports .", + "isort .", + "black .", + ] field_prefix: str = "field_" @staticmethod diff --git a/tests/test___init__.py b/tests/test___init__.py index 9a9b03313..34b05b4c5 100644 --- a/tests/test___init__.py +++ b/tests/test___init__.py @@ -5,7 +5,7 @@ import pytest import yaml -from openapi_python_client import Config, GeneratorError +from openapi_python_client import Config, ErrorLevel, GeneratorError, Project def test__get_project_for_url_or_path(mocker): @@ -241,6 +241,17 @@ def make_project(**kwargs): return Project(**kwargs) +@pytest.fixture +def project_with_dir() -> Project: + """Return a Project with the project dir pre-made (needed for cwd of commands). Unlinks after the test completes""" + project = make_project() + project.project_dir.mkdir() + + yield project + + project.project_dir.rmdir() + + class TestProject: def test___init__(self, mocker): openapi = mocker.MagicMock(title="My Test API") @@ -303,7 +314,7 @@ def test_build(self, mocker): project._build_models = mocker.MagicMock() project._build_api = mocker.MagicMock() project._create_package = mocker.MagicMock() - project._reformat = mocker.MagicMock() + project._run_post_hooks = mocker.MagicMock() project._get_errors = mocker.MagicMock() result = project.build() @@ -313,7 +324,7 @@ def test_build(self, mocker): project._build_metadata.assert_called_once() project._build_models.assert_called_once() project._build_api.assert_called_once() - project._reformat.assert_called_once() + project._run_post_hooks.assert_called_once() project._get_errors.assert_called_once() assert result == project._get_errors.return_value @@ -327,7 +338,7 @@ def test_build_no_meta(self, mocker): project._build_models = mocker.MagicMock() project._build_api = mocker.MagicMock() project._create_package = mocker.MagicMock() - project._reformat = mocker.MagicMock() + project._run_post_hooks = mocker.MagicMock() project._get_errors = mocker.MagicMock() project.build() @@ -354,7 +365,7 @@ def test_update(self, mocker): project._build_models = mocker.MagicMock() project._build_api = mocker.MagicMock() project._create_package = mocker.MagicMock() - project._reformat = mocker.MagicMock() + project._run_post_hooks = mocker.MagicMock() project._get_errors = mocker.MagicMock() result = project.update() @@ -363,7 +374,7 @@ def test_update(self, mocker): project._create_package.assert_called_once() project._build_models.assert_called_once() project._build_api.assert_called_once() - project._reformat.assert_called_once() + project._run_post_hooks.assert_called_once() project._get_errors.assert_called_once() assert result == project._get_errors.return_value @@ -501,44 +512,42 @@ def test__build_setup_py(self, mocker): setup_template.render.assert_called_once_with() setup_path.write_text.assert_called_once_with(setup_template.render(), encoding="utf-8") + def test__run_post_hooks_reports_missing_commands(self, project_with_dir): + fake_command_name = "blahblahdoesntexist" + project_with_dir.config.post_hooks = [fake_command_name] + need_to_make_cwd = not project_with_dir.project_dir.exists() + if need_to_make_cwd: + project_with_dir.project_dir.mkdir() -def test__reformat(mocker): - import subprocess + project_with_dir._run_post_hooks() - sub_run = mocker.patch("subprocess.run") - project = make_project() - project.project_dir = mocker.MagicMock(autospec=pathlib.Path) - - project._reformat() - - sub_run.assert_has_calls( - [ - mocker.call( - "autoflake -i -r --remove-all-unused-imports --remove-unused-variables --ignore-init-module-imports .", - cwd=project.package_dir, - shell=True, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - check=True, - ), - mocker.call( - "isort .", - cwd=project.project_dir, - shell=True, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - check=True, - ), - mocker.call( - "black .", - cwd=project.project_dir, - shell=True, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - check=True, - ), - ] - ) + assert len(project_with_dir.errors) == 1 + error = project_with_dir.errors[0] + assert error.level == ErrorLevel.WARNING + assert error.header == "Skipping Integration" + assert fake_command_name in error.detail + + def test__run_post_hooks_reports_stdout_of_commands_that_error_with_no_stderr(self, project_with_dir): + failing_command = "python -c \"print('a message'); exit(1)\"" + project_with_dir.config.post_hooks = [failing_command] + project_with_dir._run_post_hooks() + + assert len(project_with_dir.errors) == 1 + error = project_with_dir.errors[0] + assert error.level == ErrorLevel.ERROR + assert error.header == "python failed" + assert "a message" in error.detail + + def test__run_post_hooks_reports_stderr_of_commands_that_error(self, project_with_dir): + failing_command = "python -c \"print('a message'); raise Exception('some exception')\"" + project_with_dir.config.post_hooks = [failing_command] + project_with_dir._run_post_hooks() + + assert len(project_with_dir.errors) == 1 + error = project_with_dir.errors[0] + assert error.level == ErrorLevel.ERROR + assert error.header == "python failed" + assert "some exception" in error.detail def test__get_errors(mocker): @@ -559,7 +568,7 @@ def test__get_errors(mocker): assert project._get_errors() == [1, 2, 3] -def test__custom_templates(mocker): +def test_custom_templates(mocker): from openapi_python_client import GeneratorData, MetaType, Project openapi = mocker.MagicMock( From 87c67bb5ddd22a6ad45badccd3c21b3d6955faf9 Mon Sep 17 00:00:00 2001 From: Dylan Anthony Date: Sun, 5 Sep 2021 12:51:05 -0600 Subject: [PATCH 2/2] feat: Allow customization of post-generation steps with the `post_hooks` config option. --- README.md | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 7afb021ad..1daee8c04 100644 --- a/README.md +++ b/README.md @@ -29,12 +29,13 @@ using it (Python developers). ## Installation -I recommend you install with [pipx](https://pipxproject.github.io/pipx/) so you don't conflict with any other packages -you might have: `pipx install openapi-python-client`. +I recommend you install with [pipx](https://pipxproject.github.io/pipx/) so you don't conflict with any other packages you might have: `pipx install openapi-python-client --include-deps`. -Better yet, use `pipx run openapi-python-client ` to always use the latest version of the generator. +> Note the `--include-deps` option which will also make `black`, `isort`, and `autoflake` available in your path so that `openapi-python-client` can use them to clean up the generated code. -You can install with normal pip if you want to though: `pip install openapi-python-client` +**If you use `pipx run` then the post-generation hooks will not be available unless you install them manually.** + +You can also install with normal pip: `pip install openapi-python-client` Then, if you want tab completion: `openapi-python-client --install-completion` @@ -114,8 +115,7 @@ class_overrides: module_name: short_name ``` -The easiest way to find what needs to be overridden is probably to generate your client and go look at everything in the -models folder. +The easiest way to find what needs to be overridden is probably to generate your client and go look at everything in the models folder. ### project_name_override and package_name_override @@ -150,5 +150,16 @@ Example: package_version_override: 1.2.3 ``` +### post_hooks + +In the config file, there's an easy way to tell `openapi-python-client` to run additional commands after generation. Here's an example showing the default commands that will run if you don't override them in config: + +```yaml +post_hooks: + - "autoflake -i -r --remove-all-unused-imports --remove-unused-variables --ignore-init-module-imports ." + - "isort ." + - "black ." +``` + [changelog.md]: CHANGELOG.md [poetry]: https://python-poetry.org/