Skip to content

Commit

Permalink
Ensure we catch warnings before logger is initialized (#4351)
Browse files Browse the repository at this point in the history
  • Loading branch information
ssbarnea authored Jan 13, 2025
1 parent ec08617 commit e166961
Show file tree
Hide file tree
Showing 15 changed files with 59 additions and 28 deletions.
3 changes: 2 additions & 1 deletion .config/requirements-test.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ pydoclint
pylint
pytest
pytest-mock >= 3.10.0
pytest-plus >= 0.4.0
pytest-plus >= 0.7.0
pytest-xdist
pytest-instafail
requests != 2.32.0 # https://github.com/docker/docker-py/issues/3256
ruff
toml-sort
Expand Down
6 changes: 6 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -359,12 +359,18 @@ builtins = ["__"]
cache-dir = "./.cache/.ruff"
fix = true
line-length = 100
required-version = ">=0.7.1"
target-version = "py310"

[tool.ruff.lint]
external = [
"DOC" # pydoclint
]
ignore = [
"COM812", # conflicts with ISC001 on format
"E501", # line-too-long / rely on black
"ISC001" # conflicts with COM812 on format
]
select = ["ALL"]

[tool.ruff.lint.flake8-pytest-style]
Expand Down
29 changes: 25 additions & 4 deletions src/molecule/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,36 @@

from __future__ import annotations

from functools import lru_cache
from typing import TYPE_CHECKING

from ansible_compat.runtime import Runtime


if TYPE_CHECKING:
from pathlib import Path


class App:
"""App class that keep runtime status."""

def __init__(self) -> None:
"""Create a new app instance."""
self.runtime = Runtime(isolated=False)
def __init__(self, path: Path) -> None:
"""Create a new app instance.
Args:
path: The path to the project.
"""
self.runtime = Runtime(project_dir=path, isolated=False)


@lru_cache
def get_app(path: Path) -> App:
"""Return the app instance.
Args:
path: The path to the project.
app = App()
Returns:
App: The app instance.
"""
return App(path)
2 changes: 1 addition & 1 deletion src/molecule/command/drivers.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,6 @@ def drivers(
for driver in api.drivers().values():
description = str(driver)
if format == "plain":
description = f"{driver!s:16s}[logging.level.notset] {driver.title} Version {driver.version} from {driver.module} python module.)[/logging.level.notset]" # noqa: E501
description = f"{driver!s:16s}[logging.level.notset] {driver.title} Version {driver.version} from {driver.module} python module.)[/logging.level.notset]"
drivers.append([driver, description])
console.print(description)
2 changes: 1 addition & 1 deletion src/molecule/command/login.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ def _get_login(self, hostname: str) -> None: # pragma: no cover
login_options["lines"] = lines
if not self._config.driver.login_cmd_template:
LOG.warning(
"Login command is not supported for [dim]%s[/] host because 'login_cmd_template' was not defined in driver options.", # noqa: E501
"Login command is not supported for [dim]%s[/] host because 'login_cmd_template' was not defined in driver options.",
login_options["instance"],
)
return
Expand Down
2 changes: 1 addition & 1 deletion src/molecule/command/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ def test( # noqa: PLR0913
parallel: Whether the scenario(s) should be run in parallel mode.
ansible_args: Arguments to forward to Ansible.
platform_name: Name of the platform to use.
""" # noqa: E501
"""
args: MoleculeArgs = ctx.obj.get("args")
subcommand = base._get_subcommand(__name__) # noqa: SLF001
command_args: CommandArgs = {
Expand Down
14 changes: 7 additions & 7 deletions src/molecule/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
from packaging.version import Version

from molecule import api, interpolation, platforms, scenario, state, util
from molecule.app import app
from molecule.app import get_app
from molecule.data import __file__ as data_module
from molecule.dependency import ansible_galaxy, shell
from molecule.model import schema_v3
Expand Down Expand Up @@ -74,7 +74,7 @@ def ansible_version() -> Version:
"molecule.config.ansible_version is deprecated, will be removed in the future.",
category=DeprecationWarning,
)
return app.runtime.version
return get_app(Path()).runtime.version


class Config:
Expand Down Expand Up @@ -121,7 +121,7 @@ def __init__(
"MOLECULE_PROJECT_DIRECTORY",
os.getcwd(), # noqa: PTH109
)
self.runtime = app.runtime
self.runtime = get_app(Path(self.project_directory)).runtime
self.scenario_path = Path(molecule_file).parent

# Former after_init() contents
Expand Down Expand Up @@ -313,7 +313,7 @@ def driver(self) -> Driver:

api_drivers = api.drivers(config=self)
if driver_name not in api_drivers:
msg = f"Failed to find driver {driver_name}. Please ensure that the driver is correctly installed." # noqa: E501
msg = f"Failed to find driver {driver_name}. Please ensure that the driver is correctly installed."
util.sysexit_with_message(msg)

driver = api_drivers[driver_name]
Expand Down Expand Up @@ -395,8 +395,8 @@ def state(self) -> State:
myState.change_state("molecule_yml_date_modified", modTime)
elif myState.molecule_yml_date_modified != modTime:
LOG.warning(
"The scenario config file ('%s') has been modified since the scenario was created. " # noqa: E501
"If recent changes are important, reset the scenario with 'molecule destroy' to clean up created items or " # noqa: E501
"The scenario config file ('%s') has been modified since the scenario was created. "
"If recent changes are important, reset the scenario with 'molecule destroy' to clean up created items or "
"'molecule reset' to clear current configuration.",
self.molecule_file,
)
Expand Down Expand Up @@ -454,7 +454,7 @@ def _get_driver_name(self) -> str:
msg = (
f"Driver '{driver_name}' is currently in use but the scenario config "
f"has changed and now defines '{driver_from_scenario}'. "
"To change drivers, run 'molecule destroy' for converged scenarios or 'molecule reset' otherwise." # noqa: E501
"To change drivers, run 'molecule destroy' for converged scenarios or 'molecule reset' otherwise."
)
LOG.warning(msg)

Expand Down
2 changes: 1 addition & 1 deletion src/molecule/driver/delegated.py
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ def ansible_connection_options(
conn_dict["ansible_private_key_file"] = d.get("identity_file")
if d.get("password", None):
conn_dict["ansible_password"] = d.get("password")
# Based on testinfra documentation, ansible password must be passed via ansible_ssh_pass # noqa: E501
# Based on testinfra documentation, ansible password must be passed via ansible_ssh_pass
# issue to fix this can be found https://github.com/pytest-dev/pytest-testinfra/issues/580
conn_dict["ansible_ssh_pass"] = d.get("password")

Expand Down
4 changes: 2 additions & 2 deletions src/molecule/provisioner/ansible.py
Original file line number Diff line number Diff line change
Expand Up @@ -695,8 +695,8 @@ def inventory(self) -> dict[str, str]:
connection_options = self.connection_options(instance_name)
molecule_vars = {
"molecule_file": "{{ lookup('env', 'MOLECULE_FILE') }}",
"molecule_ephemeral_directory": "{{ lookup('env', 'MOLECULE_EPHEMERAL_DIRECTORY') }}", # noqa: E501
"molecule_scenario_directory": "{{ lookup('env', 'MOLECULE_SCENARIO_DIRECTORY') }}", # noqa: E501
"molecule_ephemeral_directory": "{{ lookup('env', 'MOLECULE_EPHEMERAL_DIRECTORY') }}",
"molecule_scenario_directory": "{{ lookup('env', 'MOLECULE_SCENARIO_DIRECTORY') }}",
"molecule_yml": "{{ lookup('file', molecule_file) | from_yaml }}",
"molecule_instance_config": "{{ lookup('env', 'MOLECULE_INSTANCE_CONFIG') }}",
"molecule_no_log": "{{ lookup('env', 'MOLECULE_NO_LOG') or not "
Expand Down
2 changes: 1 addition & 1 deletion src/molecule/provisioner/ansible_playbook.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ def execute(self, action_args: list[str] | None = None) -> str: # noqa: ARG002
from rich.markup import escape

util.sysexit_with_message(
f"Ansible return code was {result.returncode}, command was: [dim]{escape(shlex.join(result.args))}[/dim]", # noqa: E501
f"Ansible return code was {result.returncode}, command was: [dim]{escape(shlex.join(result.args))}[/dim]",
result.returncode,
warns=warns,
)
Expand Down
1 change: 0 additions & 1 deletion src/molecule/scenario.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,6 @@ def ephemeral_directory(self) -> str:
project_directory,
self.name,
)

path = ephemeral_directory(project_scenario_directory)

if isinstance(path, str):
Expand Down
6 changes: 4 additions & 2 deletions src/molecule/shell.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,16 @@
import os
import sys

from pathlib import Path

import click
import packaging

import molecule

from molecule import command, logger
from molecule.api import drivers
from molecule.app import app
from molecule.app import get_app
from molecule.command.base import click_group_ex
from molecule.config import MOLECULE_DEBUG, MOLECULE_VERBOSITY
from molecule.console import console
Expand Down Expand Up @@ -77,7 +79,7 @@ def print_version(
f"using python [repr.number]{sys.version_info[0]}.{sys.version_info[1]}[/] \n"
)

msg += f" [repr.attrib_name]ansible[/][dim]:[/][repr.number]{app.runtime.version}[/]"
msg += f" [repr.attrib_name]ansible[/][dim]:[/][repr.number]{get_app(Path()).runtime.version}[/]"
for driver in drivers().values():
msg += (
f"\n [repr.attrib_name]{driver!s}[/][dim]:[/][repr.number]{driver.version}[/][dim] "
Expand Down
6 changes: 3 additions & 3 deletions src/molecule/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
from ansible_compat.ports import cache
from rich.syntax import Syntax

from molecule.app import app
from molecule.app import get_app
from molecule.console import console
from molecule.constants import MOLECULE_HEADER

Expand Down Expand Up @@ -183,7 +183,7 @@ def run_command( # noqa: PLR0913
if debug:
print_environment_vars(env)

result = app.runtime.run(
result = get_app(Path()).runtime.run(
args=cmd,
env=env,
cwd=cwd,
Expand Down Expand Up @@ -551,7 +551,7 @@ def boolean(value: bool | AnyStr, *, strict: bool = True) -> bool:
return False

raise TypeError( # noqa: TRY003
f"The value '{value!s}' is not a valid boolean. Valid booleans include: {', '.join(repr(i) for i in BOOLEANS)!s}", # noqa: EM102, E501
f"The value '{value!s}' is not a valid boolean. Valid booleans include: {', '.join(repr(i) for i in BOOLEANS)!s}", # noqa: EM102
)


Expand Down
6 changes: 4 additions & 2 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,9 @@ def test_fixture_dir(request: pytest.FixtureRequest) -> Path:
:param request: The pytest request object
:returns: The fixture directory
"""
return FIXTURES_DIR / request.path.relative_to(Path(__file__).parent).with_suffix("")
return FIXTURES_DIR / request.path.relative_to(Path(__file__).parent).with_suffix(
"",
)


@pytest.hookimpl(hookwrapper=True, tryfirst=True)
Expand Down Expand Up @@ -198,7 +200,7 @@ def fixture_test_cache_path(
/ request.node.name,
)
if test_dir.exists():
shutil.rmtree(test_dir)
shutil.rmtree(test_dir, ignore_errors=True)

test_dir.mkdir(parents=True, exist_ok=True)

Expand Down
2 changes: 1 addition & 1 deletion tests/unit/model/v2/test_driver_section.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def _model_driver_errors_section_data_no_prefix(): # type: ignore[no-untyped-de
indirect=True,
)
def test_driver_has_errors(config): # type: ignore[no-untyped-def] # noqa: ANN001, ANN201, D103
base_error_msg = "is not one of ['azure', 'ec2', 'delegated', 'docker', 'containers', 'openstack', 'podman', 'vagrant', 'digitalocean', 'gce', 'libvirt', 'lxd', 'molecule-*', 'molecule_*', 'custom-*', 'custom_*']" # noqa: E501
base_error_msg = "is not one of ['azure', 'ec2', 'delegated', 'docker', 'containers', 'openstack', 'podman', 'vagrant', 'digitalocean', 'gce', 'libvirt', 'lxd', 'molecule-*', 'molecule_*', 'custom-*', 'custom_*']"

driver_name = str(config["driver"]["name"])
if isinstance(config["driver"]["name"], str):
Expand Down

0 comments on commit e166961

Please sign in to comment.