Skip to content

Commit

Permalink
feat: lazy-loading services (don't need a project on launch)
Browse files Browse the repository at this point in the history
  • Loading branch information
lengau committed Oct 20, 2023
1 parent 0f7d8f2 commit 38efb65
Show file tree
Hide file tree
Showing 15 changed files with 145 additions and 41 deletions.
3 changes: 2 additions & 1 deletion craft_application/services/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,15 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
"""Service classes for the business logic of various categories of command."""

from craft_application.services.base import BaseService
from craft_application.services.base import BaseService, EagerService
from craft_application.services.lifecycle import LifecycleService
from craft_application.services.package import PackageService
from craft_application.services.provider import ProviderService
from craft_application.services.service_factory import ServiceFactory

__all__ = [
"BaseService",
"EagerService",
"LifecycleService",
"PackageService",
"ProviderService",
Expand Down
53 changes: 49 additions & 4 deletions craft_application/services/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,23 +21,68 @@

from craft_cli import emit

from craft_application import errors, models

if typing.TYPE_CHECKING:
from craft_application import models
from craft_application.application import AppMetadata
from craft_application.services import ServiceFactory


# ignoring the fact that this abstract class has no abstract methods.
class BaseService(metaclass=abc.ABCMeta): # noqa: B024
"""A service containing the actual business logic of one or more commands."""
"""Abstract base class for a service. Should not be used directly.
BaseService allows lazy-loading of a project. EagerService should be used
for services that always require a project on load.
"""

def __init__(
self, app: AppMetadata, project: models.Project, services: ServiceFactory
self,
app: AppMetadata,
services: ServiceFactory,
*,
project_getter: typing.Callable[[], models.Project],
) -> None:
self._app = app
self._project = project
self._services = services
self._project: models.Project | None = None
self._project_getter = project_getter

def setup(self) -> None:
"""Application-specific service preparation."""
emit.debug("setting up service")

def _get_project(self) -> models.Project:
"""Get the project for this app run.
returns: a project model
raises: CraftError if the project cannot be loaded.
Caches the project in self._project otherwise.
"""
if self._project is None:
try:
self._project = self._project_getter()
# Intentionally catching a broad exception to provide a more useful error.
except Exception as exc: # noqa: BLE001
raise errors.CraftError(
"Project could not be loaded.", details=str(exc)
) from exc
return self._project


# ignoring the fact that this abstract class has no abstract methods.
class EagerService(BaseService, metaclass=abc.ABCMeta):
"""A service that requires the project from startup."""

_project: models.Project

def __init__(
self,
app: AppMetadata,
services: ServiceFactory,
*,
project: models.Project,
) -> None:
super().__init__(app, services, project_getter=lambda: project)
self._project = project
6 changes: 3 additions & 3 deletions craft_application/services/lifecycle.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ def _get_step(step_name: str) -> Step:
raise RuntimeError(f"Invalid target step {step_name!r}") from None


class LifecycleService(base.BaseService):
class LifecycleService(base.EagerService):
"""Create and manage the parts lifecycle.
:param app: An AppMetadata object containing metadata about the application.
Expand All @@ -116,15 +116,15 @@ class LifecycleService(base.BaseService):
def __init__( # noqa: PLR0913 (too many arguments)
self,
app: AppMetadata,
project: Project,
services: ServiceFactory,
*,
project: Project,
work_dir: Path | str,
cache_dir: Path | str,
build_for: str,
**lifecycle_kwargs: Any, # noqa: ANN401 - eventually used in an Any
) -> None:
super().__init__(app, project, services)
super().__init__(app, services, project=project)
self._work_dir = work_dir
self._cache_dir = cache_dir
self._build_for = build_for
Expand Down
2 changes: 1 addition & 1 deletion craft_application/services/package.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
from craft_application import models


class PackageService(base.BaseService):
class PackageService(base.EagerService):
"""Business logic for creating packages."""

@abc.abstractmethod
Expand Down
6 changes: 3 additions & 3 deletions craft_application/services/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
from craft_application.services import ServiceFactory


class ProviderService(base.BaseService):
class ProviderService(base.EagerService):
"""Manager for craft_providers in an application.
:param app: Metadata about this application.
Expand All @@ -55,13 +55,13 @@ class ProviderService(base.BaseService):
def __init__( # noqa: PLR0913 (too many arguments)
self,
app: AppMetadata,
project: models.Project,
services: ServiceFactory,
*,
project: models.Project,
work_dir: pathlib.Path,
install_snap: bool = True,
) -> None:
super().__init__(app, project, services)
super().__init__(app, services, project=project)
self._provider: craft_providers.Provider | None = None
self._work_dir = work_dir
self.snaps: list[Snap] = []
Expand Down
37 changes: 22 additions & 15 deletions craft_application/services/service_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,24 +71,31 @@ def __getattr__(self, service: str) -> services.BaseService:
instantiated service as an instance attribute, allowing the same service
instance to be reused for the entire run of the application.
"""
if self.project is None: # pyright: ignore[reportUnnecessaryComparison]
raise errors.ApplicationError(
"ServiceFactory requires a project to be set before getting a service.",
app_name=self.app.name,
docs_url="https://github.com/canonical/craft-application/pull/40#discussion_r1253593262",
)
service_cls_name = "".join(word.title() for word in service.split("_"))
service_cls_name += "Class"
classes = dataclasses.asdict(self)
if service_cls_name not in classes:
raise AttributeError(service)
cls = getattr(self, service_cls_name)
if issubclass(cls, services.BaseService):
kwargs = self._service_kwargs.get(service, {})
instance = cls(self.app, self.project, self, **kwargs)
instance.setup()
setattr(self, service, instance)
# Mypy and pyright interpret this differently.
# Pyright
return instance # type: ignore[no-any-return]
raise TypeError(f"{cls.__name__} is not a service class")
if not issubclass(cls, services.BaseService):
raise TypeError(f"{cls.__name__} is not a service class")

kwargs = self._service_kwargs.get(service, {})

if issubclass(cls, services.EagerService):
# Eager services require the project on load.
if self.project is None: # pyright: ignore[reportUnnecessaryComparison]
raise errors.ApplicationError(
f"{cls.__name__} requires a project to be set before loading.",
app_name=self.app.name,
docs_url="https://github.com/canonical/craft-application/pull/40#discussion_r1253593262",
)
kwargs.setdefault("project", self.project)
else:
kwargs.setdefault("project_getter", lambda: self.project)

instance = cls(self.app, self, **kwargs)
instance.setup()
setattr(self, service, instance)
# Mypy and pyright interpret this differently.
return instance # type: ignore[no-any-return]
12 changes: 7 additions & 5 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ def lifecycle_service(

service = services.LifecycleService(
app_metadata,
fake_project,
fake_services,
project=fake_project,
work_dir=work_dir,
cache_dir=cache_dir,
platform=None,
Expand All @@ -128,10 +128,11 @@ class FakeProviderService(services.ProviderService):
def __init__(
self,
app: application.AppMetadata,
project: models.Project,
services: services.ServiceFactory,
*,
project: models.Project,
):
super().__init__(app, project, services, work_dir=pathlib.Path())
super().__init__(app, services, project=project, work_dir=pathlib.Path())

return FakeProviderService

Expand Down Expand Up @@ -160,14 +161,15 @@ class FakeLifecycleService(services.LifecycleService):
def __init__(
self,
app: application.AppMetadata,
project: models.Project,
services: services.ServiceFactory,
*,
project: models.Project,
**lifecycle_kwargs: Any,
):
super().__init__(
app,
project,
services,
project=project,
work_dir=tmp_path / "work",
cache_dir=tmp_path / "cache",
platform=None,
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ def provider_service(app_metadata, fake_project, fake_services):
"""Provider service with install snap disabled for integration tests"""
return provider.ProviderService(
app_metadata,
fake_project,
fake_services,
project=fake_project,
work_dir=pathlib.Path(),
install_snap=False,
)
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/services/test_lifecycle.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ def parts_lifecycle(app_metadata, fake_project, fake_services, tmp_path, request

service = LifecycleService(
app_metadata,
fake_project,
fake_services,
project=fake_project,
work_dir=tmp_path / "work",
cache_dir=tmp_path / "cache",
platform=None,
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
@pytest.fixture()
def provider_service(app_metadata, fake_project, fake_services, tmp_path):
return services.ProviderService(
app_metadata, fake_project, fake_services, work_dir=tmp_path
app_metadata, fake_services, project=fake_project, work_dir=tmp_path
)


Expand Down
49 changes: 49 additions & 0 deletions tests/unit/services/test_base.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# This file is part of craft-application.
#
# Copyright 2023 Canonical Ltd.
#
# This program is free software: you can redistribute it and/or modify it
# under the terms of the GNU Lesser General Public License version 3, as
# published by the Free Software Foundation.
#
# This program is distributed in the hope that it will be useful, but WITHOUT
# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY,
# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE.
# See the GNU Lesser General Public License for more details.
#
# You should have received a copy of the GNU Lesser General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
"""Unit tests for base service class."""
import pytest
from craft_application import errors, services


class FakeLazyService(services.BaseService):
pass


def test_lazy_service_load_success(app_metadata, fake_services, fake_project):
def project_getter():
return fake_project

service = FakeLazyService(
app_metadata, fake_services, project_getter=project_getter
)

assert service._project is None
assert service._get_project() == fake_project
assert service._project == fake_project


def test_lazy_service_load_failure(app_metadata, fake_services):
def project_getter():
raise ValueError("No service for you!")

service = FakeLazyService(
app_metadata, fake_services, project_getter=project_getter
)

assert service._project is None
with pytest.raises(errors.CraftError, match=r"^Project could not be loaded.$"):
service._get_project()
assert service._project is None
6 changes: 3 additions & 3 deletions tests/unit/services/test_lifecycle.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ def fake_parts_lifecycle(app_metadata, fake_project, fake_services, tmp_path):
build_for = util.get_host_architecture()
fake_service = FakePartsLifecycle(
app_metadata,
fake_project,
fake_services,
project=fake_project,
work_dir=work_dir,
cache_dir=cache_dir,
build_for=build_for,
Expand Down Expand Up @@ -213,8 +213,8 @@ def test_get_step_failure(step_name):
def test_init_success(app_metadata, fake_project, fake_services, tmp_path):
service = lifecycle.LifecycleService(
app_metadata,
fake_project,
fake_services,
project=fake_project,
work_dir=tmp_path,
cache_dir=tmp_path,
platform=None,
Expand Down Expand Up @@ -246,8 +246,8 @@ def test_init_parts_error(

service = lifecycle.LifecycleService(
app_metadata,
fake_project,
fake_services,
project=fake_project,
work_dir=tmp_path,
cache_dir=tmp_path,
platform=None,
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/services/test_package.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def metadata(self) -> models.BaseMetadata:


def test_write_metadata(tmp_path, app_metadata, fake_project, fake_services):
service = FakePackageService(app_metadata, fake_project, fake_services)
service = FakePackageService(app_metadata, fake_services, project=fake_project)
metadata_file = tmp_path / "metadata.yaml"
assert not metadata_file.exists()

Expand Down
2 changes: 1 addition & 1 deletion tests/unit/services/test_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@
def test_install_snap(app_metadata, fake_project, fake_services, install_snap, snaps):
service = provider.ProviderService(
app_metadata,
fake_project,
fake_services,
project=fake_project,
work_dir=pathlib.Path(),
install_snap=install_snap,
)
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/services/test_service_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def __new__(cls, *args, **kwargs):
check.equal(factory.package, MockPackageService.mock_class.return_value)
with check:
MockPackageService.mock_class.assert_called_once_with(
app_metadata, fake_project, factory, **kwargs
app_metadata, factory, project=fake_project, **kwargs
)


Expand Down

0 comments on commit 38efb65

Please sign in to comment.