Skip to content

Commit

Permalink
pydantic: split base and charmcraft config models (#1141)
Browse files Browse the repository at this point in the history
  • Loading branch information
syu-w authored Jul 6, 2023
1 parent d244e61 commit 17ad021
Show file tree
Hide file tree
Showing 18 changed files with 122 additions and 98 deletions.
2 changes: 1 addition & 1 deletion charmcraft/bases.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

from typing import Tuple, Union

from charmcraft.models.config import Base
from charmcraft.models.charmcraft import Base
from charmcraft.utils import get_host_architecture, get_os_platform


Expand Down
2 changes: 1 addition & 1 deletion charmcraft/commands/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
UBUNTU_LTS_STABLE,
)
from charmcraft.commands.store.charmlibs import collect_charmlib_pydeps
from charmcraft.models.config import Base, BasesConfiguration
from charmcraft.models.charmcraft import Base, BasesConfiguration
from charmcraft.parts import Step
from charmcraft.utils import get_host_architecture

Expand Down
8 changes: 4 additions & 4 deletions charmcraft/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,10 @@
is_charmcraft_running_in_managed_mode,
)
from charmcraft.utils import load_yaml
from charmcraft.models.config import Config, Project
from charmcraft.models.charmcraft import CharmcraftConfig, Project


def load(dirpath: Optional[str]) -> Config:
def load(dirpath: Optional[str]) -> CharmcraftConfig:
"""Load the config from charmcraft.yaml in the indicated directory."""
if dirpath is None:
if is_charmcraft_running_in_managed_mode():
Expand All @@ -98,7 +98,7 @@ def load(dirpath: Optional[str]) -> Config:
if content is None:
# configuration is mandatory only for some commands; when not provided, it will
# be initialized all with defaults (but marked as not provided for later verification)
return Config(
return CharmcraftConfig(
type="charm",
project=Project(
dirpath=dirpath,
Expand All @@ -107,7 +107,7 @@ def load(dirpath: Optional[str]) -> Config:
),
)

return Config.unmarshal(
return CharmcraftConfig.unmarshal(
content,
project=Project(
dirpath=dirpath,
Expand Down
2 changes: 1 addition & 1 deletion charmcraft/linters.py
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ def run(self, basedir: pathlib.Path) -> str:


def analyze(
config: config.Config,
config: config.CharmcraftConfig,
basedir: pathlib.Path,
*,
override_ignore_config: bool = False,
Expand Down
4 changes: 2 additions & 2 deletions charmcraft/metafiles/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
from craft_cli import CraftError


import charmcraft.models.config
import charmcraft.models.charmcraft
import charmcraft.linters
from charmcraft.const import IMAGE_INFO_ENV_VAR

Expand All @@ -37,7 +37,7 @@
def create_manifest(
basedir: pathlib.Path,
started_at: datetime.datetime,
bases_config: Optional[charmcraft.models.config.BasesConfiguration],
bases_config: Optional[charmcraft.models.charmcraft.BasesConfiguration],
linting_results: List[charmcraft.linters.CheckResult],
) -> pathlib.Path:
"""Create manifest.yaml in basedir for given base configuration.
Expand Down
90 changes: 90 additions & 0 deletions charmcraft/models/basic.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
# Copyright 2023 Canonical Ltd.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#
# For further info, check https://github.com/canonical/charmcraft

"""Charmcraft basic pydantic model."""

import pydantic


class ModelConfigDefaults(
pydantic.BaseModel, extra=pydantic.Extra.forbid, frozen=True, validate_all=True
):
"""Define Charmcraft's defaults for the BaseModel configuration."""


class CustomStrictStr(pydantic.StrictStr):
"""Generic class to create custom strict strings validated by pydantic."""

@classmethod
def __get_validators__(cls):
"""Yield the relevant validators."""
yield from super().__get_validators__()
yield cls.custom_validate


class RelativePath(CustomStrictStr):
"""Constrained string which must be a relative path."""

@classmethod
def custom_validate(cls, value: str) -> str:
"""Validate relative path.
Check if it's an absolute path using POSIX's '/' (not os.path.sep, as the charm's
config is independent of the platform where charmcraft is running.
"""
if not value:
raise ValueError(f"{value!r} must be a valid relative path (cannot be empty)")

if value[0] == "/":
raise ValueError(f"{value!r} must be a valid relative path (cannot start with '/')")

return value


class AttributeName(CustomStrictStr):
"""Constrained string that must match the name of an attribute from linters.CHECKERS."""

@classmethod
def custom_validate(cls, value: str) -> str:
"""Validate attribute name."""
from charmcraft import linters # import here to avoid cyclic imports

valid_names = [
checker.name
for checker in linters.CHECKERS
if checker.check_type == linters.CheckType.attribute
]
if value not in valid_names:
raise ValueError(f"Bad attribute name {value!r}")
return value


class LinterName(CustomStrictStr):
"""Constrained string that must match the name of a linter from linters.CHECKERS."""

@classmethod
def custom_validate(cls, value: str) -> str:
"""Validate attribute name."""
from charmcraft import linters # import here to avoid cyclic imports

valid_names = [
checker.name
for checker in linters.CHECKERS
if checker.check_type == linters.CheckType.lint
]
if value not in valid_names:
raise ValueError(f"Bad lint name {value!r}")
return value
80 changes: 7 additions & 73 deletions charmcraft/models/config.py → charmcraft/models/charmcraft.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@
# For further info, check https://github.com/canonical/charmcraft

"""Charmcraft configuration pydantic model."""

import datetime
import pathlib
import os
import re
import keyword

from typing import Optional, List, Dict, Any

import pydantic
Expand All @@ -29,77 +29,7 @@
from charmcraft.parts import process_part_config
from charmcraft.utils import get_host_architecture
from charmcraft.format import format_pydantic_errors


class ModelConfigDefaults(
pydantic.BaseModel, extra=pydantic.Extra.forbid, frozen=True, validate_all=True
):
"""Define Charmcraft's defaults for the BaseModel configuration."""


class CustomStrictStr(pydantic.StrictStr):
"""Generic class to create custom strict strings validated by pydantic."""

@classmethod
def __get_validators__(cls):
"""Yield the relevant validators."""
yield from super().__get_validators__()
yield cls.custom_validate


class RelativePath(CustomStrictStr):
"""Constrained string which must be a relative path."""

@classmethod
def custom_validate(cls, value: str) -> str:
"""Validate relative path.
Check if it's an absolute path using POSIX's '/' (not os.path.sep, as the charm's
config is independent of the platform where charmcraft is running.
"""
if not value:
raise ValueError(f"{value!r} must be a valid relative path (cannot be empty)")

if value[0] == "/":
raise ValueError(f"{value!r} must be a valid relative path (cannot start with '/')")

return value


class AttributeName(CustomStrictStr):
"""Constrained string that must match the name of an attribute from linters.CHECKERS."""

@classmethod
def custom_validate(cls, value: str) -> str:
"""Validate attribute name."""
from charmcraft import linters # import here to avoid cyclic imports

valid_names = [
checker.name
for checker in linters.CHECKERS
if checker.check_type == linters.CheckType.attribute
]
if value not in valid_names:
raise ValueError(f"Bad attribute name {value!r}")
return value


class LinterName(CustomStrictStr):
"""Constrained string that must match the name of a linter from linters.CHECKERS."""

@classmethod
def custom_validate(cls, value: str) -> str:
"""Validate attribute name."""
from charmcraft import linters # import here to avoid cyclic imports

valid_names = [
checker.name
for checker in linters.CHECKERS
if checker.check_type == linters.CheckType.lint
]
if value not in valid_names:
raise ValueError(f"Bad lint name {value!r}")
return value
from charmcraft.models.basic import ModelConfigDefaults, AttributeName, LinterName


class CharmhubConfig(
Expand Down Expand Up @@ -157,7 +87,11 @@ class AnalysisConfig(ModelConfigDefaults, allow_population_by_field_name=True):
ignore: Ignore = Ignore()


class Config(ModelConfigDefaults, validate_all=False):
class CharmcraftConfig(
ModelConfigDefaults,
validate_all=False,
alias_generator=lambda s: s.replace("_", "-"),
):
"""Definition of charmcraft.yaml configuration."""

# this needs to go before 'parts', as it used by the validator
Expand Down
2 changes: 1 addition & 1 deletion charmcraft/providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
from craft_providers.actions.snap_installer import Snap

from charmcraft.bases import check_if_base_matches_host
from charmcraft.models.config import BasesConfiguration
from charmcraft.models.charmcraft import BasesConfiguration
from charmcraft.env import (
get_managed_environment_snap_channel,
get_managed_environment_log_path,
Expand Down
2 changes: 1 addition & 1 deletion tests/commands/test_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
from charmcraft.charm_builder import relativise
from charmcraft.bases import get_host_as_base
from charmcraft.commands.build import BUILD_DIRNAME, Builder, format_charm_file_name, launch_shell
from charmcraft.models.config import Base, BasesConfiguration
from charmcraft.models.charmcraft import Base, BasesConfiguration
from charmcraft.config import load
from charmcraft.providers import get_base_configuration
from charmcraft.utils import get_host_architecture
Expand Down
2 changes: 1 addition & 1 deletion tests/commands/test_clean.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import pytest

from charmcraft.commands.clean import CleanCommand
from charmcraft.models.config import Base, BasesConfiguration
from charmcraft.models.charmcraft import Base, BasesConfiguration
from charmcraft.utils import get_host_architecture


Expand Down
2 changes: 1 addition & 1 deletion tests/commands/test_init.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
from craft_cli import CraftError

from charmcraft.commands.init import InitCommand, DEFAULT_PROFILE, PROFILES
from charmcraft.models.config import Project
from charmcraft.models.charmcraft import Project
from charmcraft.utils import S_IXALL
from tests.test_infra import pep8_test, get_python_filepaths, pep257_test

Expand Down
2 changes: 1 addition & 1 deletion tests/commands/test_pack.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
from charmcraft.cmdbase import JSON_FORMAT
from charmcraft.commands import pack
from charmcraft.commands.pack import PackCommand, _get_charm_pack_args, _subprocess_pack_charms
from charmcraft.models.config import BasesConfiguration
from charmcraft.models.charmcraft import BasesConfiguration
from charmcraft.config import load


Expand Down
2 changes: 1 addition & 1 deletion tests/commands/test_store_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
from craft_store.errors import CredentialsUnavailable, StoreServerError

from charmcraft.cmdbase import JSON_FORMAT
from charmcraft.models.config import CharmhubConfig
from charmcraft.models.charmcraft import CharmhubConfig
from charmcraft.commands.store import (
CloseCommand,
CreateLibCommand,
Expand Down
10 changes: 5 additions & 5 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@
from craft_parts import callbacks
from craft_providers import Executor, Provider

from charmcraft import config as config_module, instrum
from charmcraft import deprecations, parts
from charmcraft.models import charmcraft as config_module
from charmcraft import deprecations, parts, instrum
from charmcraft.bases import get_host_as_base
from charmcraft.models.config import Base, BasesConfiguration
from charmcraft.models.charmcraft import Base, BasesConfiguration


@pytest.fixture(autouse=True, scope="session")
Expand All @@ -51,7 +51,7 @@ def setup_parts():
def config(tmp_path):
"""Provide a config class with an extra set method for the test to change it."""

class TestConfig(config_module.Config, frozen=False):
class TestConfig(config_module.CharmcraftConfig, frozen=False):
"""The Config, but with a method to set test values."""

def set(self, prime=None, **kwargs):
Expand Down Expand Up @@ -84,7 +84,7 @@ def set(self, prime=None, **kwargs):
def bundle_config(tmp_path):
"""Provide a config class with an extra set method for the test to change it."""

class TestConfig(config_module.Config, frozen=False):
class TestConfig(config_module.CharmcraftConfig, frozen=False):
"""The Config, but with a method to set test values."""

def set(self, prime=None, **kwargs):
Expand Down
2 changes: 1 addition & 1 deletion tests/test_bases.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

import pytest

from charmcraft.models.config import Base
from charmcraft.models.charmcraft import Base
from charmcraft.bases import check_if_base_matches_host, get_host_as_base
from charmcraft.utils import OSPlatform

Expand Down
4 changes: 2 additions & 2 deletions tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
from craft_cli import CraftError

from charmcraft import linters
from charmcraft.models.config import Base, BasesConfiguration, CharmhubConfig
from charmcraft.models.charmcraft import Base, BasesConfiguration, CharmhubConfig
from charmcraft.config import load
from charmcraft.utils import get_host_architecture

Expand Down Expand Up @@ -81,7 +81,7 @@ def test_load_managed_mode_directory(monkeypatch, tmp_path):
monkeypatch.setenv("CHARMCRAFT_MANAGED_MODE", "1")

# Patch out Config (and Project) to prevent directory validation checks.
with patch("charmcraft.config.Config"):
with patch("charmcraft.config.CharmcraftConfig"):
with patch("charmcraft.config.Project") as mock_project:
with patch("charmcraft.config.load_yaml"):
load(None)
Expand Down
2 changes: 1 addition & 1 deletion tests/test_manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
from craft_cli import CraftError

from charmcraft import __version__, linters
from charmcraft.models.config import Base, BasesConfiguration
from charmcraft.models.charmcraft import Base, BasesConfiguration
from charmcraft.metafiles.manifest import create_manifest
from charmcraft.const import IMAGE_INFO_ENV_VAR
from charmcraft.utils import OSPlatform
Expand Down
Loading

0 comments on commit 17ad021

Please sign in to comment.