Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

✨ Add TaskError exception for subprocess management #1991

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion copier/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def _handle_exceptions(method: Callable[[], None]) -> int:
except KeyboardInterrupt:
raise UserMessageError("Execution stopped by user")
except UserMessageError as error:
print(colors.red | "\n".join(error.args), file=sys.stderr)
print(colors.red | error.message, file=sys.stderr)
return 1
except UnsafeTemplateError as error:
print(colors.red | "\n".join(error.args), file=sys.stderr)
Expand Down
51 changes: 49 additions & 2 deletions copier/errors.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
"""Custom exceptions used by Copier."""

from __future__ import annotations
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer removing this line and reverting the typing changes accordingly, and address this change in a separate PR to avoid mixing concerns here.


import subprocess
import sys
from pathlib import Path
from subprocess import CompletedProcess
from typing import TYPE_CHECKING, Sequence

from .tools import printf_exception
Expand All @@ -10,6 +15,11 @@
from .template import Template
from .user_data import AnswersMap, Question

if sys.version_info < (3, 11):
from typing_extensions import Self
else:
from typing import Self


# Errors
class CopierError(Exception):
Expand All @@ -19,6 +29,12 @@ class CopierError(Exception):
class UserMessageError(CopierError):
"""Exit the program giving a message to the user."""

def __init__(self, message: str):
self.message = message

def __str__(self) -> str:
return self.message


class UnsupportedVersionError(UserMessageError):
"""Copier version does not support template version."""
Expand All @@ -40,7 +56,7 @@ def __init__(self, conf_path: Path, quiet: bool):
class MultipleConfigFilesError(ConfigFileError):
"""Both copier.yml and copier.yaml found, and that's an error."""

def __init__(self, conf_paths: "PathSeq"):
def __init__(self, conf_paths: PathSeq):
msg = str(conf_paths)
printf_exception(self, "MULTIPLE CONFIG FILES", msg=msg)
super().__init__(msg)
Expand Down Expand Up @@ -93,7 +109,7 @@ class CopierAnswersInterrupt(CopierError, KeyboardInterrupt):
"""

def __init__(
self, answers: "AnswersMap", last_question: "Question", template: "Template"
self, answers: AnswersMap, last_question: Question, template: Template
) -> None:
self.answers = answers
self.last_question = last_question
Expand All @@ -120,6 +136,37 @@ class MultipleYieldTagsError(CopierError):
"""Multiple yield tags are used in one path name, but it is not allowed."""


class TaskError(subprocess.CalledProcessError, UserMessageError):
"""Exception raised when a task fails."""

def __init__(
self,
command: str | Sequence[str],
returncode: int,
stdout: str | bytes | None,
stderr: str | bytes | None,
):
subprocess.CalledProcessError.__init__(
self, returncode=returncode, cmd=command, output=stdout, stderr=stderr
)
UserMessageError.__init__(
self,
message=f"Task {command!r} returned non-zero exit status {returncode}.",
)

@classmethod
def from_process(
cls, process: CompletedProcess[str] | CompletedProcess[bytes]
) -> Self:
"""Create a TaskError from a CompletedProcess."""
return cls(
command=process.args,
returncode=process.returncode,
stdout=process.stdout,
stderr=process.stderr,
)


# Warnings
class CopierWarning(Warning):
"""Base class for all other Copier warnings."""
Expand Down
5 changes: 4 additions & 1 deletion copier/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
from .errors import (
CopierAnswersInterrupt,
ExtensionNotFoundError,
TaskError,
UnsafeTemplateError,
UserMessageError,
YieldTagInFileError,
Expand Down Expand Up @@ -366,7 +367,9 @@ def _execute_tasks(self, tasks: Sequence[Task]) -> None:

extra_env = {k.upper(): str(v) for k, v in task.extra_vars.items()}
with local.cwd(working_directory), local.env(**extra_env):
subprocess.run(task_cmd, shell=use_shell, check=True, env=local.env)
process = subprocess.run(task_cmd, shell=use_shell, env=local.env)
if process.returncode:
raise TaskError.from_process(process)

def _system_render_context(self) -> AnyByStrMutableMapping:
"""System reserved render context.
Expand Down
2 changes: 1 addition & 1 deletion poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ pyyaml = ">=5.3.1"
questionary = ">=1.8.1"
eval-type-backport = { version = ">=0.1.3,<0.3.0", python = "<3.10" }
platformdirs = ">=4.3.6"
typing-extensions = { version = ">=4.0.0,<5.0.0", python = "<3.11" }

[tool.poetry.group.dev]
optional = true
Expand All @@ -59,7 +60,6 @@ types-backports = ">=0.1.3"
types-colorama = ">=0.4"
types-pygments = ">=2.17"
types-pyyaml = ">=6.0.4"
typing-extensions = { version = ">=3.10.0.0,<5.0.0", python = "<3.10" }

[tool.poetry.group.docs]
optional = true
Expand Down
7 changes: 3 additions & 4 deletions tests/test_cleanup.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
from pathlib import Path
from subprocess import CalledProcessError

import pytest
from plumbum import local
Expand All @@ -10,15 +9,15 @@
def test_cleanup(tmp_path: Path) -> None:
"""Copier creates dst_path, fails to copy and removes it."""
dst = tmp_path / "new_folder"
with pytest.raises(CalledProcessError):
with pytest.raises(copier.errors.TaskError):
copier.run_copy("./tests/demo_cleanup", dst, quiet=True, unsafe=True)
assert not dst.exists()


def test_do_not_cleanup(tmp_path: Path) -> None:
"""Copier creates dst_path, fails to copy and keeps it."""
dst = tmp_path / "new_folder"
with pytest.raises(CalledProcessError):
with pytest.raises(copier.errors.TaskError):
copier.run_copy(
"./tests/demo_cleanup", dst, quiet=True, unsafe=True, cleanup_on_error=False
)
Expand All @@ -29,7 +28,7 @@ def test_no_cleanup_when_folder_existed(tmp_path: Path) -> None:
"""Copier will not delete a folder if it didn't create it."""
preexisting_file = tmp_path / "something"
preexisting_file.touch()
with pytest.raises(CalledProcessError):
with pytest.raises(copier.errors.TaskError):
copier.run_copy(
"./tests/demo_cleanup",
tmp_path,
Expand Down