Skip to content

Commit

Permalink
refactor: move parts plugins to their own module (#1897)
Browse files Browse the repository at this point in the history
This separates parts plugins from other things we do with craft-parts.
  • Loading branch information
lengau committed Sep 12, 2024
1 parent 1d2413a commit 392f859
Show file tree
Hide file tree
Showing 11 changed files with 72 additions and 37 deletions.
12 changes: 8 additions & 4 deletions charmcraft/application/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@
import craft_application
import craft_cli
from craft_application import util
from craft_parts.plugins import plugins
from craft_parts.plugins.plugins import PluginType
from overrides import override

from charmcraft import extensions, models, preprocess, services
from charmcraft.application import commands
from charmcraft.parts import BundlePlugin, CharmPlugin, ReactivePlugin
from charmcraft.parts import plugins
from charmcraft.services import CharmcraftServiceFactory

GENERAL_SUMMARY = """
Expand Down Expand Up @@ -131,8 +131,12 @@ def _get_dispatcher(self) -> craft_cli.Dispatcher:
return self._dispatcher

@override
def _get_app_plugins(self) -> dict[str, plugins.PluginType]:
return {"charm": CharmPlugin, "bundle": BundlePlugin, "reactive": ReactivePlugin}
def _get_app_plugins(self) -> dict[str, PluginType]:
return {
"charm": plugins.CharmPlugin,
"bundle": plugins.BundlePlugin,
"reactive": plugins.ReactivePlugin,
}

@override
def _pre_run(self, dispatcher: craft_cli.Dispatcher) -> None:
Expand Down
27 changes: 14 additions & 13 deletions charmcraft/parts/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright 2021-2023 Canonical Ltd.
# Copyright 2021-2024 Canonical Ltd.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
Expand All @@ -18,19 +18,14 @@

from typing import Any

from craft_parts import plugins
import craft_parts
from craft_parts.parts import PartSpec

from charmcraft.parts.bundle import BundlePlugin
from charmcraft.parts.charm import CharmPlugin, CharmPluginProperties
from . import plugins
from charmcraft.parts.lifecycle import PartsLifecycle
from charmcraft.parts.reactive import ReactivePlugin, ReactivePluginProperties

__all__ = [
"CharmPlugin",
"CharmPluginProperties",
"ReactivePlugin",
"ReactivePluginProperties",
"plugins",
"setup_parts",
"process_part_config",
"PartsLifecycle",
Expand All @@ -39,7 +34,13 @@

def setup_parts():
"""Initialize craft-parts plugins."""
plugins.register({"charm": CharmPlugin, "bundle": BundlePlugin, "reactive": ReactivePlugin})
craft_parts.plugins.register(
{
"charm": plugins.CharmPlugin,
"bundle": plugins.BundlePlugin,
"reactive": plugins.ReactivePlugin,
}
)


def process_part_config(data: dict[str, Any]) -> dict[str, Any]:
Expand All @@ -59,18 +60,18 @@ def process_part_config(data: dict[str, Any]) -> dict[str, Any]:
if not plugin_name:
raise ValueError("'plugin' not defined")

plugin_class = plugins.get_plugin_class(plugin_name)
plugin_class = craft_parts.plugins.get_plugin_class(plugin_name)

# validate plugin properties
plugin_properties = plugin_class.properties_class.unmarshal(spec)

# validate common part properties
part_spec = plugins.extract_part_properties(spec, plugin_name=plugin_name)
part_spec = craft_parts.plugins.extract_part_properties(spec, plugin_name=plugin_name)
PartSpec(**part_spec)

# get plugin properties data if it's model based (otherwise it's empty), and
# update with the received config
if isinstance(plugin_properties, plugins.PluginProperties):
if isinstance(plugin_properties, craft_parts.plugins.PluginProperties):
full_config = plugin_properties.model_dump(by_alias=True, exclude_unset=True)
else:
full_config = {}
Expand Down
30 changes: 30 additions & 0 deletions charmcraft/parts/plugins/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# Copyright 2024 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

"""Craft-parts plugins and plugin overrides for charmcraft."""

from ._bundle import BundlePlugin, BundlePluginProperties
from ._charm import CharmPlugin, CharmPluginProperties
from ._reactive import ReactivePlugin, ReactivePluginProperties

__all__ = [
"BundlePlugin",
"BundlePluginProperties",
"CharmPlugin",
"CharmPluginProperties",
"ReactivePlugin",
"ReactivePluginProperties",
]
File renamed without changes.
File renamed without changes.
File renamed without changes.
4 changes: 2 additions & 2 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ def charm_plugin(tmp_path):
"charm-python-packages": ["pkg3", "pkg4"],
"charm-requirements": requirement_files,
}
plugin_properties = parts.CharmPluginProperties.unmarshal(spec)
plugin_properties = charmcraft.parts.plugins.CharmPluginProperties.unmarshal(spec)
part_spec = plugins.extract_part_properties(spec, plugin_name="charm")
part = craft_parts.Part(
"foo", part_spec, project_dirs=project_dirs, plugin_properties=plugin_properties
Expand All @@ -370,7 +370,7 @@ def bundle_plugin(tmp_path):
"plugin": "bundle",
"source": str(tmp_path),
}
plugin_properties = charmcraft.parts.bundle.BundlePluginProperties.unmarshal(spec)
plugin_properties = charmcraft.parts.plugins.BundlePluginProperties.unmarshal(spec)
part_spec = plugins.extract_part_properties(spec, plugin_name="bundle")
part = craft_parts.Part(
"foo", part_spec, project_dirs=project_dirs, plugin_properties=plugin_properties
Expand Down
Empty file.
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ def test_charmplugin_post_build_metric_collection(charm_plugin):
def test_charmpluginproperties_invalid_properties():
content = {"source": ".", "charm-invalid": True}
with pytest.raises(pydantic.ValidationError) as raised:
parts.CharmPlugin.properties_class.unmarshal(content)
parts.plugins.CharmPlugin.properties_class.unmarshal(content)
err = raised.value.errors()

assert len(err) == 1
Expand All @@ -216,22 +216,22 @@ def test_charmpluginproperties_invalid_properties():
def test_charmpluginproperties_entrypoint_ok():
"""Simple valid entrypoint."""
content = {"source": ".", "charm-entrypoint": "myep.py"}
properties = parts.CharmPlugin.properties_class.unmarshal(content)
properties = parts.plugins.CharmPlugin.properties_class.unmarshal(content)
assert properties.charm_entrypoint == "myep.py"


def test_charmpluginproperties_entrypoint_default():
"""Specific default if not configured."""
content = {"source": "."}
properties = parts.CharmPlugin.properties_class.unmarshal(content)
properties = parts.plugins.CharmPlugin.properties_class.unmarshal(content)
assert properties.charm_entrypoint == "src/charm.py"


def test_charmpluginproperties_entrypoint_relative(tmp_path):
"""The configuration is stored relative no matter what."""
absolute_path = tmp_path / "myep.py"
content = {"source": str(tmp_path), "charm-entrypoint": str(absolute_path)}
properties = parts.CharmPlugin.properties_class.unmarshal(content)
properties = parts.plugins.CharmPlugin.properties_class.unmarshal(content)
assert properties.charm_entrypoint == "myep.py"


Expand All @@ -240,7 +240,7 @@ def test_charmpluginproperties_entrypoint_outside_project_absolute(tmp_path):
outside_path = tmp_path.parent / "charm.py"
content = {"source": str(tmp_path), "charm-entrypoint": str(outside_path)}
with pytest.raises(pydantic.ValidationError) as raised:
parts.CharmPlugin.properties_class.unmarshal(content)
parts.plugins.CharmPlugin.properties_class.unmarshal(content)
err = raised.value.errors()
assert len(err) == 1
assert err[0]["loc"] == ("charm-entrypoint",)
Expand All @@ -255,7 +255,7 @@ def test_charmpluginproperties_entrypoint_outside_project_relative(tmp_path):
outside_path = tmp_path.parent / "charm.py"
content = {"source": str(tmp_path), "charm-entrypoint": "../charm.py"}
with pytest.raises(pydantic.ValidationError) as raised:
parts.CharmPlugin.properties_class.unmarshal(content)
parts.plugins.CharmPlugin.properties_class.unmarshal(content)
err = raised.value.errors()
assert len(err) == 1
assert err[0]["loc"] == ("charm-entrypoint",)
Expand All @@ -268,15 +268,15 @@ def test_charmpluginproperties_entrypoint_outside_project_relative(tmp_path):
def test_charmpluginproperties_requirements_default(tmp_path):
"""The configuration is empty by default."""
content = {"source": str(tmp_path)}
properties = parts.CharmPlugin.properties_class.unmarshal(content)
properties = parts.plugins.CharmPlugin.properties_class.unmarshal(content)
assert properties.charm_requirements == []


def test_charmpluginproperties_requirements_filepresent_ok(tmp_path: pathlib.Path):
"""If a specific file is present in disk it's used."""
(tmp_path / "requirements.txt").write_text("somedep")
content = {"source": str(tmp_path)}
properties = parts.CharmPluginProperties.unmarshal(content)
properties = parts.plugins.CharmPluginProperties.unmarshal(content)
assert properties.charm_requirements == ["requirements.txt"]


Expand All @@ -285,5 +285,5 @@ def test_charmpluginproperties_requirements_filepresent_but_configured(tmp_path)
(tmp_path / "requirements.txt").write_text("somedep")
(tmp_path / "alternative.txt").write_text("somedep")
content = {"source": str(tmp_path), "charm-requirements": ["alternative.txt"]}
properties = parts.CharmPlugin.properties_class.unmarshal(content)
properties = parts.plugins.CharmPlugin.properties_class.unmarshal(content)
assert properties.charm_requirements == ["alternative.txt"]
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
from craft_parts.errors import PluginEnvironmentValidationError

from charmcraft import const
from charmcraft.parts import reactive
from charmcraft.parts.plugins import _reactive

pytestmark = pytest.mark.skipif(sys.platform == "win32", reason="Windows not [yet] supported")

Expand Down Expand Up @@ -69,7 +69,7 @@ def spec(tmp_path):

@pytest.fixture
def plugin_properties(spec):
return reactive.ReactivePluginProperties.unmarshal(spec)
return _reactive.ReactivePluginProperties.unmarshal(spec)


@pytest.fixture
Expand Down Expand Up @@ -104,7 +104,7 @@ def test_get_build_environment(plugin):

def test_get_build_commands(plugin, tmp_path):
assert plugin.get_build_commands() == [
f"{sys.executable} -I {reactive.__file__} fake-project "
f"{sys.executable} -I {_reactive.__file__} fake-project "
f"{tmp_path}/parts/foo/build {tmp_path}/parts/foo/install "
"--charm-argument --charm-argument-with argument"
]
Expand Down Expand Up @@ -178,7 +178,7 @@ def fake_run():

def test_build(build_dir, install_dir, fake_run):
fake_run.return_value = CompletedProcess(("charm", "build"), 0)
returncode = reactive.build(
returncode = _reactive.build(
charm_name="test-charm",
build_dir=build_dir,
install_dir=install_dir,
Expand Down Expand Up @@ -207,7 +207,7 @@ def test_build(build_dir, install_dir, fake_run):
def test_build_charm_proof_raises_error_messages(build_dir, install_dir, fake_run):
fake_run.side_effect = CalledProcessError(200, "E: name missing")

returncode = reactive.build(
returncode = _reactive.build(
charm_name="test-charm",
build_dir=build_dir,
install_dir=install_dir,
Expand All @@ -226,7 +226,7 @@ def test_build_charm_proof_raises_warning_messages_does_not_raise(
):
fake_run.side_effect = CalledProcessError(100, "W: Description is not pretty")

returncode = reactive.build(
returncode = _reactive.build(
charm_name="test-charm",
build_dir=build_dir,
install_dir=install_dir,
Expand Down Expand Up @@ -266,7 +266,7 @@ def _run_generator():

fake_run.side_effect = _run_generator()

returncode = reactive.build(
returncode = _reactive.build(
charm_name="test-charm",
build_dir=build_dir,
install_dir=install_dir,
Expand All @@ -292,7 +292,7 @@ def _run_generator():
]

# Also ensure negative return codes raises error
returncode = reactive.build(
returncode = _reactive.build(
charm_name="test-charm",
build_dir=build_dir,
install_dir=install_dir,
Expand All @@ -315,7 +315,7 @@ def _run_generator():

fake_run.side_effect = _run_generator()

returncode = reactive.build(
returncode = _reactive.build(
charm_name="test-charm",
build_dir=build_dir,
install_dir=install_dir,
Expand Down

0 comments on commit 392f859

Please sign in to comment.