Skip to content

Commit

Permalink
Change cache_dir location to prefer venv and project_directory (#439)
Browse files Browse the repository at this point in the history
  • Loading branch information
ssbarnea authored Jan 9, 2025
1 parent a9387df commit 29d0329
Show file tree
Hide file tree
Showing 10 changed files with 120 additions and 83 deletions.
16 changes: 1 addition & 15 deletions .config/pydoclint-baseline.txt
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,6 @@ src/ansible_compat/loaders.py
DOC501: Function `colpath_from_path` has "raise" statements, but the docstring does not have a "Raises" section
DOC503: Function `colpath_from_path` exceptions in the "Raises" section in the docstring do not match those in the function body Raises values in the docstring: []. Raised exceptions in the body: ['InvalidPrerequisiteError'].
--------------------
src/ansible_compat/prerun.py
DOC101: Function `get_cache_dir`: Docstring contains fewer arguments than in function signature.
DOC103: Function `get_cache_dir`: Docstring arguments are different from function arguments. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Arguments in the function signature but not in the docstring: [project_dir: Path].
DOC201: Function `get_cache_dir` does not have a return section in docstring
--------------------
src/ansible_compat/runtime.py
DOC601: Class `Collection`: Class docstring contains fewer class attributes than actual class attributes. (Please read https://jsh9.github.io/pydoclint/checking_class_attributes.html on how to correctly document class attributes.)
DOC603: Class `Collection`: Class docstring attributes are different from actual class attributes. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Attributes in the class definition but not in the docstring: [name: str, path: Path, version: str]. (Please read https://jsh9.github.io/pydoclint/checking_class_attributes.html on how to correctly document class attributes.)
Expand All @@ -69,7 +64,7 @@ src/ansible_compat/runtime.py
DOC501: Method `Plugins.__getattribute__` has "raise" statements, but the docstring does not have a "Raises" section
DOC503: Method `Plugins.__getattribute__` exceptions in the "Raises" section in the docstring do not match those in the function body Raises values in the docstring: []. Raised exceptions in the body: ['AnsibleCompatError'].
DOC601: Class `Runtime`: Class docstring contains fewer class attributes than actual class attributes. (Please read https://jsh9.github.io/pydoclint/checking_class_attributes.html on how to correctly document class attributes.)
DOC603: Class `Runtime`: Class docstring attributes are different from actual class attributes. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Attributes in the class definition but not in the docstring: [_has_playbook_cache: dict[tuple[str, Path | None], bool], _version: Version | None, cache_dir: Path | None, collections: OrderedDict[str, Collection], initialized: bool, plugins: Plugins, require_module: bool]. (Please read https://jsh9.github.io/pydoclint/checking_class_attributes.html on how to correctly document class attributes.)
DOC603: Class `Runtime`: Class docstring attributes are different from actual class attributes. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Attributes in the class definition but not in the docstring: [_has_playbook_cache: dict[tuple[str, Path | None], bool], _version: Version | None, cache_dir: Path, collections: OrderedDict[str, Collection], initialized: bool, plugins: Plugins, require_module: bool]. (Please read https://jsh9.github.io/pydoclint/checking_class_attributes.html on how to correctly document class attributes.)
DOC101: Method `Runtime.__init__`: Docstring contains fewer arguments than in function signature.
DOC103: Method `Runtime.__init__`: Docstring arguments are different from function arguments. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Arguments in the function signature but not in the docstring: [environ: dict[str, str] | None, isolated: bool, max_retries: int, min_required_version: str | None, project_dir: Path | None, require_module: bool, verbosity: int].
DOC501: Method `Runtime.__init__` has "raise" statements, but the docstring does not have a "Raises" section
Expand Down Expand Up @@ -110,8 +105,6 @@ src/ansible_compat/runtime.py
DOC501: Method `Runtime._prepare_ansible_paths` has "raise" statements, but the docstring does not have a "Raises" section
DOC503: Method `Runtime._prepare_ansible_paths` exceptions in the "Raises" section in the docstring do not match those in the function body Raises values in the docstring: []. Raised exceptions in the body: ['RuntimeError'].
DOC201: Method `Runtime._get_roles_path` does not have a return section in docstring
DOC101: Method `Runtime._install_galaxy_role`: Docstring contains fewer arguments than in function signature.
DOC103: Method `Runtime._install_galaxy_role`: Docstring arguments are different from function arguments. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Arguments in the function signature but not in the docstring: [project_dir: Path].
DOC501: Method `Runtime._install_galaxy_role` has "raise" statements, but the docstring does not have a "Raises" section
DOC503: Method `Runtime._install_galaxy_role` exceptions in the "Raises" section in the docstring do not match those in the function body Raises values in the docstring: []. Raised exceptions in the body: ['InvalidPrerequisiteError'].
DOC101: Method `Runtime._update_env`: Docstring contains fewer arguments than in function signature.
Expand Down Expand Up @@ -285,13 +278,6 @@ test/test_runtime.py
DOC101: Function `test_prepare_environment_symlink`: Docstring contains fewer arguments than in function signature.
DOC103: Function `test_prepare_environment_symlink`: Docstring arguments are different from function arguments. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Arguments in the function signature but not in the docstring: [caplog: pytest.LogCaptureFixture, dest: str | Path, message: str].
--------------------
test/test_runtime_scan_path.py
DOC601: Class `ScanSysPath`: Class docstring contains fewer class attributes than actual class attributes. (Please read https://jsh9.github.io/pydoclint/checking_class_attributes.html on how to correctly document class attributes.)
DOC603: Class `ScanSysPath`: Class docstring attributes are different from actual class attributes. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Attributes in the class definition but not in the docstring: [raises_not_found: bool, scan: bool]. (Please read https://jsh9.github.io/pydoclint/checking_class_attributes.html on how to correctly document class attributes.)
DOC201: Method `ScanSysPath.__str__` does not have a return section in docstring
DOC101: Function `test_scan_sys_path`: Docstring contains fewer arguments than in function signature.
DOC103: Function `test_scan_sys_path`: Docstring arguments are different from function arguments. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Arguments in the function signature but not in the docstring: [monkeypatch: MonkeyPatch, param: ScanSysPath, runtime_tmp: Runtime, tmp_path: Path, venv_module: VirtualEnvironment].
--------------------
test/test_schema.py
DOC101: Function `json_from_asset`: Docstring contains fewer arguments than in function signature.
DOC103: Function `json_from_asset`: Docstring arguments are different from function arguments. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Arguments in the function signature but not in the docstring: [file_name: str].
Expand Down
9 changes: 9 additions & 0 deletions .taplo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[formatting]
# compatibility between toml-sort-fix pre-commit hook and panekj.even-betterer-toml extension
align_comments = false
array_trailing_comma = false
compact_arrays = true
compact_entries = false
compact_inline_tables = true
inline_table_expand = false
reorder_keys = true
13 changes: 10 additions & 3 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
{
"[json]": {
"editor.defaultFormatter": "esbenp.prettier-vscode"
},
"[jsonc]": {
"editor.defaultFormatter": "esbenp.prettier-vscode"
},
Expand All @@ -14,8 +17,6 @@
"editor.formatOnSave": true
},
"editor.formatOnSave": true,
"evenBetterToml.formatter.alignComments": false,
"evenBetterToml.formatter.allowedBlankLines": 2,
"files.exclude": {
"*.egg-info": true,
".pytest_cache": true,
Expand All @@ -37,8 +38,14 @@
"python.testing.pytestArgs": ["tests"],
"python.testing.pytestEnabled": true,
"python.testing.unittestEnabled": false,
"sortLines.filterBlankLines": true,
"yaml.completion": true,
"yaml.customTags": ["!encrypted/pkcs1-oaep scalar", "!vault scalar"],
"yaml.format.enable": false,
"yaml.validate": true
"yaml.validate": true,
"evenBetterToml.formatter.alignComments": false,
"evenBetterToml.formatter.arrayTrailingComma": true,
"[toml]": {
"editor.defaultFormatter": "panekj.even-betterer-toml"
}
}
19 changes: 17 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,16 @@ known-first-party = ["ansible_compat"]
known-third-party = ["packaging"]

[tool.ruff.lint.per-file-ignores]
"test/**/*.py" = ["DOC402", "DOC501", "SLF001", "S101", "S404", "FBT001", "PLC2701"]
"test/**/*.py" = [
"DOC402",
"DOC501",
"FBT001",
"PLC2701",
"PLR0917",
"S101",
"S404",
"SLF001"
]

[tool.ruff.lint.pydocstyle]
convention = "google"
Expand Down Expand Up @@ -430,4 +439,10 @@ sort_table_keys = true
[tool.uv.pip]
annotation-style = "line"
custom-compile-command = "tox run deps"
no-emit-package = ["ansible-core", "pip", "resolvelib", "typing_extensions", "uv"]
no-emit-package = [
"ansible-core",
"pip",
"resolvelib",
"typing_extensions",
"uv"
]
37 changes: 22 additions & 15 deletions src/ansible_compat/prerun.py
Original file line number Diff line number Diff line change
@@ -1,22 +1,29 @@
"""Utilities for configuring ansible runtime environment."""

import hashlib
import os
from pathlib import Path


def get_cache_dir(project_dir: Path) -> Path:
"""Compute cache directory to be used based on project path."""
# we only use the basename instead of the full path in order to ensure that
# we would use the same key regardless the location of the user home
# directory or where the project is clones (as long the project folder uses
# the same name).
basename = project_dir.resolve().name.encode(encoding="utf-8")
# 6 chars of entropy should be enough
cache_key = hashlib.sha256(basename).hexdigest()[:6]
cache_dir = (
Path(os.getenv("XDG_CACHE_HOME", "~/.cache")).expanduser()
/ "ansible-compat"
/ cache_key
)
def get_cache_dir(project_dir: Path, *, isolated: bool = True) -> Path:
"""Compute cache directory to be used based on project path.
Args:
project_dir: Path to the project directory.
isolated: Whether to use isolated cache directory.
Returns:
Cache directory path.
"""
if "VIRTUAL_ENV" in os.environ:
cache_dir = Path(os.environ["VIRTUAL_ENV"]) / ".ansible"
elif isolated:
cache_dir = project_dir / ".ansible"
else:
cache_dir = Path(os.environ.get("ANSIBLE_HOME", "~/.ansible")).expanduser()

# Ensure basic folder structure exists so `ansible-galaxy list` does not
# fail with: None of the provided paths were usable. Please specify a valid path with
for name in ("roles", "collections"): # pragma: no cover
(cache_dir / name).mkdir(parents=True, exist_ok=True)

return cache_dir
20 changes: 7 additions & 13 deletions src/ansible_compat/runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ class Runtime:

_version: Version | None = None
collections: OrderedDict[str, Collection] = OrderedDict()
cache_dir: Path | None = None
cache_dir: Path
# Used to track if we have already initialized the Ansible runtime as attempts
# to do it multiple tilmes will cause runtime warnings from within ansible-core
initialized: bool = False
Expand Down Expand Up @@ -209,8 +209,8 @@ def __init__(
if "PYTHONWARNINGS" not in self.environ: # pragma: no cover
self.environ["PYTHONWARNINGS"] = "ignore:Blowfish has been deprecated"

if isolated:
self.cache_dir = get_cache_dir(self.project_dir)
self.cache_dir = get_cache_dir(self.project_dir, isolated=self.isolated)

self.config = AnsibleConfig(cache_dir=self.cache_dir)

# Add the sys.path to the collection paths if not isolated
Expand Down Expand Up @@ -356,8 +356,7 @@ def _ensure_module_available(self) -> None:

def clean(self) -> None:
"""Remove content of cache_dir."""
if self.cache_dir:
shutil.rmtree(self.cache_dir, ignore_errors=True)
shutil.rmtree(self.cache_dir, ignore_errors=True)

def run( # ruff: disable=PLR0913
self,
Expand Down Expand Up @@ -567,8 +566,7 @@ def install_requirements( # noqa: C901
]
if self.verbosity > 0:
cmd.extend(["-" + ("v" * self.verbosity)])
if self.cache_dir:
cmd.extend(["--roles-path", f"{self.cache_dir}/roles"])
cmd.extend(["--roles-path", f"{self.cache_dir}/roles"])

if offline:
_logger.warning(
Expand Down Expand Up @@ -668,8 +666,7 @@ def prepare_environment( # noqa: C901
destination=destination,
)

if self.cache_dir:
destination = self.cache_dir / "collections"
destination = self.cache_dir / "collections"
for name, min_version in required_collections.items():
self.install_collection(
f"{name}:>={min_version}",
Expand Down Expand Up @@ -837,10 +834,7 @@ def _get_roles_path(self) -> Path:
not mentioned or set to `False`, it returns the first path in
`default_roles_path`.
"""
if self.cache_dir:
path = Path(f"{self.cache_dir}/roles")
else:
path = Path(self.config.default_roles_path[0]).expanduser()
path = Path(f"{self.cache_dir}/roles")
return path

def _install_galaxy_role(
Expand Down
28 changes: 28 additions & 0 deletions test/test_prerun.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
"""Tests for ansible_compat.prerun module."""

from __future__ import annotations

from pathlib import Path
from typing import TYPE_CHECKING

if TYPE_CHECKING:
from _pytest.monkeypatch import MonkeyPatch

from ansible_compat.prerun import get_cache_dir

Expand All @@ -10,3 +16,25 @@ def test_get_cache_dir_relative() -> None:
relative_path = Path()
abs_path = relative_path.resolve()
assert get_cache_dir(relative_path) == get_cache_dir(abs_path)


def test_get_cache_dir_no_isolation_no_venv(monkeypatch: MonkeyPatch) -> None:
"""Test behaviors of get_cache_dir.
Args:
monkeypatch: Pytest fixture for monkeypatching
"""
monkeypatch.delenv("VIRTUAL_ENV", raising=False)
monkeypatch.delenv("ANSIBLE_HOME", raising=False)
assert get_cache_dir(Path(), isolated=False) == Path("~/.ansible").expanduser()


def test_get_cache_dir_isolation_no_venv(monkeypatch: MonkeyPatch) -> None:
"""Test behaviors of get_cache_dir.
Args:
monkeypatch: Pytest fixture for monkeypatching
"""
monkeypatch.delenv("VIRTUAL_ENV", raising=False)
monkeypatch.delenv("ANSIBLE_HOME", raising=False)
assert get_cache_dir(Path(), isolated=True) == Path() / ".ansible"
15 changes: 7 additions & 8 deletions test/test_runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,11 +175,6 @@ def test_runtime_install_role(
f"{Path(runtime.config.default_roles_path[0]).expanduser()}/{role_name}",
).is_symlink()
runtime.clean()
# also test that clean does not break when cache_dir is missing
tmp_dir = runtime.cache_dir
runtime.cache_dir = None
runtime.clean()
runtime.cache_dir = tmp_dir


def test_prepare_environment_with_collections(runtime_tmp: Runtime) -> None:
Expand Down Expand Up @@ -654,10 +649,9 @@ def test_upgrade_collection(runtime_tmp: Runtime) -> None:
runtime_tmp.require_collection("community.molecule", "0.1.0")


def test_require_collection_no_cache_dir() -> None:
def test_require_collection_not_isolated() -> None:
"""Check require_collection without a cache directory."""
runtime = Runtime()
assert not runtime.cache_dir
runtime = Runtime(isolated=False)
runtime.require_collection("community.molecule", "0.1.0", install=True)


Expand Down Expand Up @@ -1024,6 +1018,11 @@ def test_runtime_has_playbook() -> None:
"""Tests has_playbook method."""
runtime = Runtime(require_module=True)

runtime.prepare_environment(
required_collections={"community.molecule": "0.1.0"},
install_local=True,
)

assert not runtime.has_playbook("this-does-not-exist.yml")
# call twice to ensure cache is used:
assert not runtime.has_playbook("this-does-not-exist.yml")
Expand Down
45 changes: 18 additions & 27 deletions test/test_runtime_scan_path.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import json
import textwrap
from dataclasses import dataclass, fields
from pathlib import Path

import pytest
Expand All @@ -19,26 +18,11 @@
V2_COLLECTION_FULL_NAME = f"{V2_COLLECTION_NAMESPACE}.{V2_COLLECTION_NAME}"


@dataclass
class ScanSysPath:
"""Parameters for scan tests."""

scan: bool
raises_not_found: bool

def __str__(self) -> str:
"""Return a string representation of the object."""
parts = [
f"{field.name}{str(getattr(self, field.name))[0]}" for field in fields(self)
]
return "-".join(parts)


@pytest.mark.parametrize(
("param"),
("scan", "raises_not_found"),
(
ScanSysPath(scan=False, raises_not_found=True),
ScanSysPath(scan=True, raises_not_found=False),
pytest.param(False, True, id="0"),
pytest.param(True, False, id="1"),
),
ids=str,
)
Expand All @@ -47,16 +31,23 @@ def test_scan_sys_path(
monkeypatch: MonkeyPatch,
runtime_tmp: Runtime,
tmp_path: Path,
param: ScanSysPath,
scan: bool,
raises_not_found: bool,
) -> None:
"""Confirm sys path is scanned for collections.
:param venv_module: Fixture for a virtual environment
:param monkeypatch: Fixture for monkeypatching
:param runtime_tmp: Fixture for a Runtime object
:param tmp_dir: Fixture for a temporary directory
:param param: The parameters for the test
Args:
venv_module: Fixture for a virtual environment
monkeypatch: Fixture for monkeypatching
runtime_tmp: Fixture for a Runtime object
tmp_path: Fixture for a temporary directory
scan: Whether to scan the sys path
raises_not_found: Whether the collection is expected to be found
"""
# Isolated the test from the others, so ansible will not find collections
# that might be installed by other tests.
monkeypatch.setenv("VIRTUAL_ENV", venv_module.project.as_posix())
monkeypatch.setenv("ANSIBLE_HOME", tmp_path.as_posix())
first_site_package_dir = venv_module.site_package_dirs()[0]

installed_to = (
Expand All @@ -76,7 +67,7 @@ def test_scan_sys_path(
# Confirm the collection is installed
assert installed_to.exists()
# Set the sys scan path environment variable
monkeypatch.setenv("ANSIBLE_COLLECTIONS_SCAN_SYS_PATH", str(param.scan))
monkeypatch.setenv("ANSIBLE_COLLECTIONS_SCAN_SYS_PATH", str(scan))
# Set the ansible collections paths to avoid bleed from other tests
monkeypatch.setenv("ANSIBLE_COLLECTIONS_PATH", str(tmp_path))

Expand All @@ -91,7 +82,7 @@ def test_scan_sys_path(
)

proc = venv_module.python_script_run(script)
if param.raises_not_found:
if raises_not_found:
assert proc.returncode != 0, (proc.stdout, proc.stderr)
assert "InvalidPrerequisiteError" in proc.stderr
assert "'community.molecule' not found" in proc.stderr
Expand Down
1 change: 1 addition & 0 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ passenv =
LANG
LC_*
setenv =
ANSIBLE_HOME = {envdir}/.ansible
ANSIBLE_DEVEL_WARNING='false'
COVERAGE_FILE = {env:COVERAGE_FILE:{envdir}/.coverage.{envname}}
COVERAGE_PROCESS_START={toxinidir}/pyproject.toml
Expand Down

0 comments on commit 29d0329

Please sign in to comment.