From 9c2f73af8cf37559dd82487de0731f9806806673 Mon Sep 17 00:00:00 2001 From: Evgeniy Alekseev Date: Tue, 28 Jun 2022 11:55:48 +0300 Subject: [PATCH] simplify tmpdir method --- CONTRIBUTING.md | 2 +- .../application/application_packages.py | 6 +++-- src/ahriman/core/repository/executor.py | 6 +++-- src/ahriman/core/tree.py | 6 +++-- src/ahriman/core/util.py | 27 +------------------ tests/ahriman/core/test_tree.py | 4 --- tests/ahriman/core/test_util.py | 21 +-------------- 7 files changed, 15 insertions(+), 57 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 69682d13..4cb7391d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -31,7 +31,7 @@ Again, the most checks can be performed by `make check` command, though some add * In case if class load requires some actions, it is recommended to create class method which can be used for class instantiating. * The code must follow the exception safety, unless it is explicitly asked by end user. It means that most exceptions must be handled and printed to log, no other actions must be done (e.g. raising another exception). * For the external command `ahriman.core.util.check_output` function must be used. -* Every temporary file/directory must be removed at the end of processing, no matter what. The `ahriman.core.util.tmpdir` function provides wrapper for the directories. +* Every temporary file/directory must be removed at the end of processing, no matter what. The ``tempfile`` module provides good ways to do it. * Import order must be the following: ```python diff --git a/src/ahriman/application/application/application_packages.py b/src/ahriman/application/application/application_packages.py index 0d2e0f81..559612d6 100644 --- a/src/ahriman/application/application/application_packages.py +++ b/src/ahriman/application/application/application_packages.py @@ -21,11 +21,12 @@ import requests import shutil from pathlib import Path +from tempfile import TemporaryDirectory from typing import Any, Iterable, Set from ahriman.application.application.application_properties import ApplicationProperties from ahriman.core.build_tools.sources import Sources -from ahriman.core.util import package_like, tmpdir +from ahriman.core.util import package_like from ahriman.models.package import Package from ahriman.models.package_source import PackageSource from ahriman.models.result import Result @@ -85,7 +86,8 @@ class ApplicationPackages(ApplicationProperties): self.database.build_queue_insert(package) self.database.remote_update(package) - with tmpdir() as local_dir: + with TemporaryDirectory(ignore_cleanup_errors=True) as dir_name, \ + (local_dir := Path(dir_name)): # pylint: disable=confusing-with-statement Sources.load(local_dir, package, self.database.patches_get(package.base), self.repository.paths) self._process_dependencies(local_dir, known_packages, without_dependencies) diff --git a/src/ahriman/core/repository/executor.py b/src/ahriman/core/repository/executor.py index 4c3c1bb6..2e5c5a6b 100644 --- a/src/ahriman/core/repository/executor.py +++ b/src/ahriman/core/repository/executor.py @@ -20,11 +20,12 @@ import shutil from pathlib import Path +from tempfile import TemporaryDirectory from typing import Iterable, List, Optional, Set from ahriman.core.build_tools.task import Task from ahriman.core.repository.cleaner import Cleaner -from ahriman.core.util import safe_filename, tmpdir +from ahriman.core.util import safe_filename from ahriman.models.package import Package from ahriman.models.package_description import PackageDescription from ahriman.models.result import Result @@ -83,7 +84,8 @@ class Executor(Cleaner): result = Result() for single in updates: - with tmpdir() as build_dir: + with TemporaryDirectory(ignore_cleanup_errors=True) as dir_name, \ + (build_dir := Path(dir_name)): # pylint: disable=confusing-with-statement try: build_single(single, build_dir) result.add_success(single) diff --git a/src/ahriman/core/tree.py b/src/ahriman/core/tree.py index 75376998..c737abeb 100644 --- a/src/ahriman/core/tree.py +++ b/src/ahriman/core/tree.py @@ -19,11 +19,12 @@ # from __future__ import annotations +from pathlib import Path +from tempfile import TemporaryDirectory from typing import Iterable, List, Set, Type from ahriman.core.build_tools.sources import Sources from ahriman.core.database import SQLite -from ahriman.core.util import tmpdir from ahriman.models.package import Package from ahriman.models.repository_paths import RepositoryPaths @@ -71,7 +72,8 @@ class Leaf: Returns: Leaf: loaded class """ - with tmpdir() as clone_dir: + with TemporaryDirectory(ignore_cleanup_errors=True) as dir_name, \ + (clone_dir := Path(dir_name)): # pylint: disable=confusing-with-statement Sources.load(clone_dir, package, database.patches_get(package.base), paths) dependencies = Package.dependencies(clone_dir) return cls(package, dependencies) diff --git a/src/ahriman/core/util.py b/src/ahriman/core/util.py index 060ebe4b..2153c53f 100644 --- a/src/ahriman/core/util.py +++ b/src/ahriman/core/util.py @@ -22,11 +22,8 @@ import io import os import re import requests -import shutil import subprocess -import tempfile -from contextlib import contextmanager from enum import Enum from logging import Logger from pathlib import Path @@ -37,7 +34,7 @@ from ahriman.models.repository_paths import RepositoryPaths __all__ = ["check_output", "check_user", "exception_response_text", "filter_json", "full_version", "enum_values", - "package_like", "pretty_datetime", "pretty_size", "safe_filename", "tmpdir", "walk"] + "package_like", "pretty_datetime", "pretty_size", "safe_filename", "walk"] def check_output(*args: str, exception: Optional[Exception], cwd: Optional[Path] = None, @@ -294,28 +291,6 @@ def safe_filename(source: str) -> str: return re.sub(r"[^A-Za-z\d\-._~:\[\]@]", "-", source) -@contextmanager -def tmpdir() -> Generator[Path, None, None]: - """ - wrapper for tempfile to remove directory after all - - Yields: - Path: path to the created directory - - Examples: - This function must be used only inside context manager as decorator states:: - - >>> with tmpdir() as path: - >>> do_something(path) - >>> raise Exception("Clear me after exception please") - """ - path = Path(tempfile.mkdtemp()) - try: - yield path - finally: - shutil.rmtree(path, ignore_errors=True) - - def walk(directory_path: Path) -> Generator[Path, None, None]: """ list all file paths in given directory diff --git a/tests/ahriman/core/test_tree.py b/tests/ahriman/core/test_tree.py index 939cdc86..402cab1a 100644 --- a/tests/ahriman/core/test_tree.py +++ b/tests/ahriman/core/test_tree.py @@ -43,19 +43,15 @@ def test_leaf_load(package_ahriman: Package, repository_paths: RepositoryPaths, """ must load with dependencies """ - tempdir_mock = mocker.patch("tempfile.mkdtemp") load_mock = mocker.patch("ahriman.core.build_tools.sources.Sources.load") dependencies_mock = mocker.patch("ahriman.models.package.Package.dependencies", return_value={"ahriman-dependency"}) - rmtree_mock = mocker.patch("shutil.rmtree") leaf = Leaf.load(package_ahriman, repository_paths, database) assert leaf.package == package_ahriman assert leaf.dependencies == {"ahriman-dependency"} - tempdir_mock.assert_called_once_with() load_mock.assert_called_once_with( pytest.helpers.anyvar(int), package_ahriman, None, repository_paths) dependencies_mock.assert_called_once_with(pytest.helpers.anyvar(int)) - rmtree_mock.assert_called_once_with(pytest.helpers.anyvar(int), ignore_errors=True) def test_tree_levels(leaf_ahriman: Leaf, leaf_python_schedule: Leaf) -> None: diff --git a/tests/ahriman/core/test_util.py b/tests/ahriman/core/test_util.py index 5d1dda11..b3199144 100644 --- a/tests/ahriman/core/test_util.py +++ b/tests/ahriman/core/test_util.py @@ -10,7 +10,7 @@ from unittest.mock import MagicMock from ahriman.core.exceptions import BuildFailed, InvalidOption, UnsafeRun from ahriman.core.util import check_output, check_user, exception_response_text, filter_json, full_version, \ - enum_values, package_like, pretty_datetime, pretty_size, safe_filename, tmpdir, walk + enum_values, package_like, pretty_datetime, pretty_size, safe_filename, walk from ahriman.models.package import Package from ahriman.models.package_source import PackageSource from ahriman.models.repository_paths import RepositoryPaths @@ -307,25 +307,6 @@ def test_safe_filename() -> None: assert safe_filename("tolua++-1.0.93-4-x86_64.pkg.tar.zst") == "tolua---1.0.93-4-x86_64.pkg.tar.zst" -def test_tmpdir() -> None: - """ - must create temporary directory and remove it after - """ - with tmpdir() as directory: - assert directory.is_dir() - assert not directory.exists() - - -def test_tmpdir_failure() -> None: - """ - must create temporary directory and remove it even after exception - """ - with pytest.raises(Exception): - with tmpdir() as directory: - raise Exception() - assert not directory.exists() - - def test_walk(resource_path_root: Path) -> None: """ must traverse directory recursively