Skip to content

Commit

Permalink
fix: issues showing lines for plugin config validation errors (#2157)
Browse files Browse the repository at this point in the history
  • Loading branch information
antazoey authored Jun 20, 2024
1 parent 7c6a530 commit c153052
Show file tree
Hide file tree
Showing 2 changed files with 102 additions and 6 deletions.
84 changes: 79 additions & 5 deletions src/ape/api/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,19 @@ class MyConfig(PluginConfig):
"""


def _find_config_yaml_files(base_path: Path) -> list[Path]:
"""
Find all ape config file in the given path.
"""
found: list[Path] = []
if (base_path / "ape-config.yaml").is_file():
found.append(base_path / "ape-config.yaml")
if (base_path / "ape-config.yml").is_file():
found.append(base_path / "ape-config.yml")

return found


class PluginConfig(BaseSettings):
"""
A base plugin configuration class. Each plugin that includes
Expand All @@ -56,7 +69,9 @@ class PluginConfig(BaseSettings):
model_config = SettingsConfigDict(extra="allow")

@classmethod
def from_overrides(cls, overrides: dict) -> "PluginConfig":
def from_overrides(
cls, overrides: dict, plugin_name: Optional[str] = None, project_path: Optional[Path] = None
) -> "PluginConfig":
default_values = cls().model_dump()

def update(root: dict, value_map: dict):
Expand All @@ -72,7 +87,54 @@ def update(root: dict, value_map: dict):
try:
return cls.model_validate(data)
except ValidationError as err:
raise ConfigError(str(err)) from err
plugin_name = plugin_name or cls.__name__.replace("Config", "").lower()
if problems := cls._find_plugin_config_problems(
err, plugin_name, project_path=project_path
):
raise ConfigError(problems) from err
else:
raise ConfigError(str(err)) from err

@classmethod
def _find_plugin_config_problems(
cls, err: ValidationError, plugin_name: str, project_path: Optional[Path] = None
) -> Optional[str]:
# Attempt showing line-nos for failed plugin config validation.
# This is trickier than root-level data since by this time, we
# no longer are aware of which files are responsible for which config.
ape = ManagerAccessMixin

# First, try checking the root config file ALONE. It is important to do
# w/o any data from the project-level config to isolate the source of the problem.
raw_global_data = ape.config_manager.global_config.model_dump(by_alias=True)
if plugin_name in raw_global_data:
try:
cls.model_validate(raw_global_data[plugin_name])
except Exception:
if problems := cls._find_plugin_config_problems_from_file(
err, ape.config_manager.DATA_FOLDER
):
return problems

# No issues found with global; try the local project.
# NOTE: No need to isolate project-data w/o root-data because we have already
# determined root-level data is OK.
project_path = project_path or ape.local_project.path
if problems := cls._find_plugin_config_problems_from_file(err, project_path):
return problems

return None

@classmethod
def _find_plugin_config_problems_from_file(
cls, err: ValidationError, base_path: Path
) -> Optional[str]:
cfg_files = _find_config_yaml_files(base_path)
for cfg_file in cfg_files:
if problems := _get_problem_with_config(err.errors(), cfg_file):
return problems

return None

@only_raise_attribute_error
def __getattr__(self, attr_name: str) -> Any:
Expand Down Expand Up @@ -221,6 +283,12 @@ class ApeConfig(ExtraAttributesMixin, BaseSettings, ManagerAccessMixin):
The top-level config.
"""

def __init__(self, *args, **kwargs):
project_path = kwargs.get("project")
super(BaseSettings, self).__init__(*args, **kwargs)
# NOTE: Cannot reference `self` at all until after super init.
self._project_path = project_path

contracts_folder: Optional[str] = None
"""
The path to the folder containing the contract source files.
Expand Down Expand Up @@ -437,7 +505,9 @@ def get_plugin_config(self, name: str) -> Optional[PluginConfig]:

if cls != ConfigDict:
# NOTE: Will raise if improperly provided keys
config = cls.from_overrides(cfg)
config = cls.from_overrides(
cfg, plugin_name=plugin_name, project_path=self._project_path
)
else:
# NOTE: Just use it directly as a dict if `ConfigDict` is passed
config = cfg
Expand Down Expand Up @@ -470,15 +540,19 @@ def get_custom_ecosystem_config(self, name: str) -> Optional[PluginConfig]:
from ape_ethereum import EthereumConfig

ethereum = cast(EthereumConfig, self.get_plugin_config("ethereum"))
return ethereum.from_overrides(override)
return ethereum.from_overrides(
override, plugin_name=name, project_path=self._project_path
)

return None

def get_unknown_config(self, name: str) -> PluginConfig:
# This happens when a plugin is not installed but still configured.
result = (self.__pydantic_extra__ or {}).get(name, PluginConfig())
if isinstance(result, dict):
return PluginConfig.from_overrides(result)
return PluginConfig.from_overrides(
result, plugin_name=name, project_path=self._project_path
)

return result

Expand Down
24 changes: 23 additions & 1 deletion tests/functional/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,15 @@ def test_config_access():
)


def test_plugin_config_updates_when_default_is_empty_dict():
def test_from_overrides():
class MyConfig(PluginConfig):
foo: int = 0

actual = MyConfig.from_overrides({"foo": 1})
assert actual.foo == 1


def test_from_overrides_updates_when_default_is_empty_dict():
class SubConfig(PluginConfig):
foo: int = 0
bar: int = 1
Expand All @@ -256,6 +264,20 @@ class MyConfig(PluginConfig):
assert actual.sub == {"baz": {"test": SubConfig(foo=5, bar=1)}}


def test_from_overrides_shows_errors_in_project_config():
class MyConfig(PluginConfig):
foo: int = 0

with create_tempdir() as tmp_path:
file = tmp_path / "ape-config.yaml"
file.write_text("foo: [1,2,3]")

with pytest.raises(ConfigError) as err:
_ = MyConfig.from_overrides({"foo": [1, 2, 3]}, project_path=tmp_path)

assert "-->1: foo: [1,2,3]" in str(err.value)


@pytest.mark.parametrize("override_0,override_1", [(True, {"foo": 0}), ({"foo": 0}, True)])
def test_plugin_config_with_union_dicts(override_0, override_1):
class SubConfig(PluginConfig):
Expand Down

0 comments on commit c153052

Please sign in to comment.