fix: Don't crash the generator when one of the post-generation hooks is missing [fixes #479]. Thanks @chamini2 and @karolzlot!

Author: dbanty

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:

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

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(