Skip to content

Commit

Permalink
feat: show config error linenos (#2134)
Browse files Browse the repository at this point in the history
  • Loading branch information
antazoey authored Jun 13, 2024
1 parent a7a501c commit 114781d
Show file tree
Hide file tree
Showing 4 changed files with 126 additions and 4 deletions.
18 changes: 17 additions & 1 deletion src/ape/_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@
from typing import Any, Optional

import click
import rich
import yaml

from ape.cli import ape_cli_context
from ape.exceptions import Abort, ApeException, handle_ape_exception
from ape.exceptions import Abort, ApeException, ConfigError, handle_ape_exception
from ape.logging import logger
from ape.plugins._utils import PluginMetadataList, clean_plugin_name
from ape.utils.basemodel import ManagerAccessMixin
Expand All @@ -35,10 +36,25 @@ def display_config(ctx, param, value):
ctx.exit() # NOTE: Must exit to bypass running ApeCLI


def _validate_config():
try:
_ = ManagerAccessMixin.local_project.config
except ConfigError as err:
rich.print(err)
# Exit now to avoid weird problems.
sys.exit(1)


class ApeCLI(click.MultiCommand):
_commands: Optional[dict] = None
_CLI_GROUP_NAME = "ape_cli_subcommands"

def __init__(self, *args, **kwargs):
# Validate the config before any argument parsing,
# as arguments may utilize config.
_validate_config()
super().__init__(*args, **kwargs)

def format_commands(self, ctx, formatter) -> None:
commands = []
for subcommand in self.list_commands(ctx):
Expand Down
72 changes: 69 additions & 3 deletions src/ape/api/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@
from typing import TYPE_CHECKING, Any, Optional, TypeVar, cast

import yaml
from ethpm_types import PackageManifest, PackageMeta
from pydantic import ConfigDict, Field, model_validator
from ethpm_types import PackageManifest, PackageMeta, Source
from pydantic import ConfigDict, Field, ValidationError, model_validator
from pydantic_settings import BaseSettings, SettingsConfigDict

from ape.exceptions import ConfigError
from ape.logging import logger
from ape.types import AddressType
from ape.utils import clean_path
from ape.utils.basemodel import (
ExtraAttributesMixin,
ExtraModelAttributes,
Expand Down Expand Up @@ -274,7 +275,72 @@ def validate_file(cls, path: Path, **overrides) -> "ApeConfig":
# relative from.
data["project"] = path.parent

return cls.model_validate(data)
try:
return cls.model_validate(data)
except ValidationError as err:
if path.suffix == ".json":
# TODO: Support JSON configs here.
raise # The validation error as-is

# Several errors may have been collected from the config.
all_errors = err.errors()
# Attempt to find line numbers in the config matching.
cfg_content = Source(content=path.read_text(encoding="utf8")).content
if not cfg_content:
# Likely won't happen?
raise # The validation error as-is

err_map: dict = {}
for error in all_errors:
if not (location := error.get("loc")):
continue

lineno = None
loc_idx = 0
depth = len(location)
for no, line in cfg_content.items():
loc = location[loc_idx]
if not line.lstrip().startswith(f"{loc}:"):
continue

# Look for the next location up the tree.
loc_idx += 1
if loc_idx < depth:
continue

# Found.
lineno = no
break

if lineno is not None and loc_idx >= depth:
err_map[lineno] = error
# else: we looped through the whole file and didn't find anything.

if not err_map:
# raise ValidationError as-is (no line numbers found for some reason).
raise

error_strs: list[str] = []
for line_no, cfg_err in err_map.items():
sub_message = cfg_err.get("msg", cfg_err)
line_before = line_no - 1 if line_no > 1 else None
line_after = line_no + 1 if line_no < len(cfg_content) else None
lines = []
if line_before is not None:
lines.append(f" {line_before}: {cfg_content[line_before]}")
lines.append(f"-->{line_no}: {cfg_content[line_no]}")
if line_after is not None:
lines.append(f" {line_after}: {cfg_content[line_after]}")

file_preview = "\n".join(lines)
sub_message = f"{sub_message}\n{file_preview}\n"
error_strs.append(sub_message)

# NOTE: Using reversed because later pydantic errors
# appear earlier in the list.
final_msg = "\n".join(reversed(error_strs)).strip()
final_msg = f"'{clean_path(path)}' is invalid!\n{final_msg}"
raise ConfigError(final_msg)

@classmethod
def from_manifest(cls, manifest: PackageManifest, **overrides) -> "ApeConfig":
Expand Down
14 changes: 14 additions & 0 deletions src/ape/managers/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -2431,6 +2431,20 @@ def clean(self):
self._manifest = PackageManifest()

self.sources._path_cache = None
self._clear_cached_config()

def reload_config(self):
"""
Reload the local ape-config.yaml file.
This is useful if the file was modified in the
active python session.
"""
self._clear_cached_config()
_ = self.config

def _clear_cached_config(self):
if "config" in self.__dict__:
del self.__dict__["config"]

def _create_contract_source(self, contract_type: ContractType) -> Optional[ContractSource]:
if not (source_id := contract_type.source_id):
Expand Down
26 changes: 26 additions & 0 deletions tests/integration/cli/test_misc.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import os
import re

import pytest

from ape import Project
from tests.conftest import ApeSubprocessRunner
from tests.integration.cli.utils import run_once


Expand Down Expand Up @@ -30,3 +33,26 @@ def test_help(ape_cli, runner):
rf"test\s*Launches pytest{anything}"
)
assert re.match(expected.strip(), result.output)


@run_once
def test_invalid_config():
# Using subprocess runner so we re-hit the init of the cmd.
runner = ApeSubprocessRunner("ape")
here = os.curdir
with Project.create_temporary_project() as tmp:
cfgfile = tmp.path / "ape-config.yaml"
# Name is invalid!
cfgfile.write_text("name:\n {asdf}")

os.chdir(tmp.path)
result = runner.invoke("--help")
os.chdir(here)

expected = """
Input should be a valid string
-->1: name:
2: {asdf}
""".strip()
assert result.exit_code != 0
assert expected in result.output

0 comments on commit 114781d

Please sign in to comment.