From e52128348a779395dcd2b1dcca7c70019a0d59d4 Mon Sep 17 00:00:00 2001 From: Debojit Kaushik Date: Wed, 17 Jun 2026 14:14:17 -0400 Subject: [PATCH 1/4] Added StorageFactory and VariableFactory. Registered StorageService and VariableService to LFX service discovery routine to return service objects, and not None. Internal method calls still call noop, but avoids disgraceful errors such as AttributeError(s). --- src/lfx/src/lfx/schema/image.py | 27 +++ src/lfx/src/lfx/services/initialize.py | 20 ++- src/lfx/src/lfx/services/storage/factory.py | 37 ++++ src/lfx/src/lfx/services/variable/factory.py | 27 +++ src/lfx/src/lfx/utils/image.py | 8 +- .../unit/services/test_default_factories.py | 159 ++++++++++++++++++ 6 files changed, 272 insertions(+), 6 deletions(-) create mode 100644 src/lfx/src/lfx/services/storage/factory.py create mode 100644 src/lfx/src/lfx/services/variable/factory.py create mode 100644 src/lfx/tests/unit/services/test_default_factories.py diff --git a/src/lfx/src/lfx/schema/image.py b/src/lfx/src/lfx/schema/image.py index f160732b5681..c0cc4502ba2c 100644 --- a/src/lfx/src/lfx/schema/image.py +++ b/src/lfx/src/lfx/schema/image.py @@ -82,6 +82,13 @@ def get_file_paths(files: list[str | dict]): if not file_path_str: # Skip empty paths continue + # An absolute path is a real filesystem path, not a logical + # "flow_id/filename" storage key; keep it as-is so downstream readers + # open it directly instead of rebuilding it under data_dir. + if Path(file_path_str).is_absolute(): + file_paths.append(file_path_str) + continue + flow_id, file_name = storage_service.parse_file_path(file_path_str) if not file_name: # Skip if no filename @@ -132,6 +139,26 @@ async def get_files( if not file: # Skip empty file paths continue + # An absolute path that exists on disk is a real filesystem file, not a + # logical "flow_id/filename" storage key. Read it directly so plain + # local paths work even when a storage service is registered (routing + # them through the service would parse flow_id="/abs/dir" and fail). + # Non-existent absolute paths still fall through to the storage service, + # which owns paths under its data_dir. + local_path = Path(file) + if local_path.is_absolute() and local_path.exists(): + try: + async with aiofiles.open(local_path, "rb") as f: + file_content = await f.read() + except Exception as e: + msg = f"Error reading file {file}: {e}" + raise FileNotFoundError(msg) from e + if convert_to_base64: + file_objects.append(base64.b64encode(file_content).decode("utf-8")) + else: + file_objects.append(file_content) + continue + flow_id, file_name = storage_service.parse_file_path(file) if not file_name: # Skip if no filename diff --git a/src/lfx/src/lfx/services/initialize.py b/src/lfx/src/lfx/services/initialize.py index 35b158f64fa6..a088ca10804e 100644 --- a/src/lfx/src/lfx/services/initialize.py +++ b/src/lfx/src/lfx/services/initialize.py @@ -1,21 +1,31 @@ """Initialize services for lfx package.""" from lfx.services.settings.factory import SettingsServiceFactory +from lfx.services.storage.factory import StorageServiceFactory +from lfx.services.variable.factory import VariableServiceFactory def initialize_services(): """Initialize required services for lfx.""" from lfx.services.manager import get_service_manager - # Register the settings service factory service_manager = get_service_manager() + + # Register the lean no-deps defaults. Settings is the only hard requirement; + # storage and variable are registered here so file-backed and variable-using + # components have a real service in bare lfx instead of None. These go into + # the factory tier, so a heavier backend (e.g. langflow) can override either + # through the same manager (config/decorator registrations take precedence). service_manager.register_factory(SettingsServiceFactory()) + service_manager.register_factory(StorageServiceFactory()) + service_manager.register_factory(VariableServiceFactory()) - # Ensure built-in pluggable services are registered (decorator runs on import). - # This allows LFX to use minimal auth/telemetry/tracing/variable when no config overrides. + # Note: auth and authorization self-register at import time via the + # @register_service decorator. Storage and variable do NOT self-register on + # import — they are wired up by the explicit register_factory calls above. - # Note: We don't create the service immediately, - # it will be created on first use via get_settings_service() + # Services are created lazily on first use (e.g. via get_storage_service()), + # not eagerly here. # Initialize services when the module is imported diff --git a/src/lfx/src/lfx/services/storage/factory.py b/src/lfx/src/lfx/services/storage/factory.py new file mode 100644 index 000000000000..c70f0e8dee9d --- /dev/null +++ b/src/lfx/src/lfx/services/storage/factory.py @@ -0,0 +1,37 @@ +"""Factory for the lean local storage service used by bare lfx.""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from typing_extensions import override + +from lfx.services.factory import ServiceFactory +from lfx.services.schema import ServiceType +from lfx.services.storage.local import LocalStorageService + +if TYPE_CHECKING: + from lfx.services.settings.service import SettingsService + + +class StorageServiceFactory(ServiceFactory): + """Registers ``LocalStorageService`` as the no-deps default storage backend. + + This is the lean default for bare lfx: it depends only on the settings + service (always available) so any file-backed component has a real storage + service instead of ``None``. A heavier backend (e.g. langflow) overrides it + through the same service manager by registering its own storage factory or + service class. + """ + + def __init__(self) -> None: + super().__init__() + self.service_class = LocalStorageService + self.dependencies = [ServiceType.SETTINGS_SERVICE] + + @override + def create(self, settings_service: SettingsService) -> LocalStorageService: + # Bare lfx has no session service. LocalStorageService only needs the + # settings service for its data_dir; session_service is stored but never + # used by any file operation, so passing None here is safe. + return LocalStorageService(session_service=None, settings_service=settings_service) diff --git a/src/lfx/src/lfx/services/variable/factory.py b/src/lfx/src/lfx/services/variable/factory.py new file mode 100644 index 000000000000..5734259f2939 --- /dev/null +++ b/src/lfx/src/lfx/services/variable/factory.py @@ -0,0 +1,27 @@ +"""Factory for the lean env-only variable service used by bare lfx.""" + +from __future__ import annotations + +from typing_extensions import override + +from lfx.services.factory import ServiceFactory +from lfx.services.variable.service import VariableService + + +class VariableServiceFactory(ServiceFactory): + """Registers the env-only ``VariableService`` as the no-deps default. + + This is the lean default for bare lfx: it has no dependencies and provides + in-memory + environment-variable resolution so variable-using components get + a real service instead of ``None``. A heavier backend (e.g. langflow) can + override it through the same service manager with a database-backed variant. + """ + + def __init__(self) -> None: + super().__init__() + self.service_class = VariableService + self.dependencies = [] + + @override + def create(self) -> VariableService: + return VariableService() diff --git a/src/lfx/src/lfx/utils/image.py b/src/lfx/src/lfx/utils/image.py index 0b12c218201f..0eefed376e95 100644 --- a/src/lfx/src/lfx/utils/image.py +++ b/src/lfx/src/lfx/utils/image.py @@ -28,7 +28,13 @@ def convert_image_to_base64(image_path: str | Path) -> str: """ image_path = Path(image_path) - storage_service = get_storage_service() + # An absolute path is a real filesystem path, not a logical + # "flow_id/filename" storage key. Read it directly so callers can pass plain + # local paths (e.g. tempfiles) regardless of whether a storage service is + # registered. Routing such a path through the storage service would parse it + # into flow_id="/abs/dir" and raise a validation error instead of the + # expected FileNotFoundError when the file is missing. + storage_service = None if image_path.is_absolute() else get_storage_service() if storage_service: flow_id, file_name = storage_service.parse_file_path(str(image_path)) try: diff --git a/src/lfx/tests/unit/services/test_default_factories.py b/src/lfx/tests/unit/services/test_default_factories.py new file mode 100644 index 000000000000..cf12cc094426 --- /dev/null +++ b/src/lfx/tests/unit/services/test_default_factories.py @@ -0,0 +1,159 @@ +"""Tests that bare lfx registers lean default storage/variable factories. + +Without these factories, ``get_storage_service()`` / ``get_variable_service()`` +return ``None`` in a process without langflow and file-backed or variable-using +components hit ``AttributeError``. These tests verify the defaults are wired up, +usable, and overridable through the same service manager. +""" + +import lfx.services.manager as manager_mod +import pytest +from lfx.services.factory import ServiceFactory +from lfx.services.manager import ServiceManager +from lfx.services.schema import ServiceType +from lfx.services.storage.factory import StorageServiceFactory +from lfx.services.storage.local import LocalStorageService +from lfx.services.variable.factory import VariableServiceFactory +from lfx.services.variable.service import VariableService + + +@pytest.fixture +def fresh_service_manager(monkeypatch): + """Swap the global service manager for a fresh one (auto-restored).""" + import asyncio + + new_manager = ServiceManager() + monkeypatch.setattr(manager_mod, "_service_manager", new_manager) + yield new_manager + asyncio.run(new_manager.teardown()) + + +def test_factories_carry_expected_metadata(): + """The lean defaults are no-deps (variable) / settings-only (storage).""" + storage_factory = StorageServiceFactory() + variable_factory = VariableServiceFactory() + + assert storage_factory.service_class is LocalStorageService + assert storage_factory.dependencies == [ServiceType.SETTINGS_SERVICE] + + assert variable_factory.service_class is VariableService + assert variable_factory.dependencies == [] + + +def test_get_storage_and_variable_services_are_not_none(fresh_service_manager): + """In a process without langflow the deps helpers return real services.""" + from lfx.services.deps import get_storage_service, get_variable_service + from lfx.services.initialize import initialize_services + + initialize_services() + + storage = get_storage_service() + variables = get_variable_service() + + assert storage is not None + assert isinstance(storage, LocalStorageService) + assert variables is not None + assert isinstance(variables, VariableService) + + # The deps helpers resolve through the (isolated) global manager. + assert fresh_service_manager.get(ServiceType.STORAGE_SERVICE) is storage + assert fresh_service_manager.get(ServiceType.VARIABLE_SERVICE) is variables + + +@pytest.mark.asyncio +async def test_default_storage_service_saves_and_reads_file(fresh_service_manager, monkeypatch, tmp_path): + """A file-backed flow can save and read a file via the default storage service.""" + monkeypatch.setenv("LANGFLOW_CONFIG_DIR", str(tmp_path)) + + from lfx.services.deps import get_storage_service + from lfx.services.initialize import initialize_services + + initialize_services() + assert ServiceType.STORAGE_SERVICE.value in fresh_service_manager.factories + + storage = get_storage_service() + assert storage is not None # would be None (AttributeError on use) before the fix + + await storage.save_file("flow_1", "data.txt", b"hello") + assert await storage.get_file("flow_1", "data.txt") == b"hello" + + +@pytest.mark.asyncio +async def test_default_storage_service_rejects_traversal(fresh_service_manager, monkeypatch, tmp_path): + """The factory-created storage service blocks path traversal on every entry point. + + Insurance against a future refactor re-introducing the traversal issue on a + subset of methods: the service is built the same way production builds it + (through the factory + manager), and a sentinel file planted outside the data + dir must never be touched. + """ + data_dir = tmp_path / "config" + data_dir.mkdir() + monkeypatch.setenv("LANGFLOW_CONFIG_DIR", str(data_dir)) + + # Sentinel outside data_dir that a successful traversal would clobber. + sentinel = tmp_path / "secret.txt" + sentinel.write_text("original") + + from lfx.services.deps import get_storage_service + from lfx.services.initialize import initialize_services + + initialize_services() + storage = get_storage_service() + assert isinstance(storage, LocalStorageService) + assert fresh_service_manager.get(ServiceType.STORAGE_SERVICE) is storage + + # flow_id carrying traversal / separators / null bytes is rejected. + for bad_flow_id in ("../secret", "..", "a/b", "a\\b", "evil\x00"): + with pytest.raises(ValueError, match=r"flow_id|path traversal"): + await storage.save_file(bad_flow_id, "secret.txt", b"pwned") + + # file_name carrying traversal / separators is rejected too. + for bad_file_name in ("../secret.txt", "a/b.txt", "a\\b.txt"): + with pytest.raises(ValueError, match=r"file name|path traversal"): + await storage.save_file("flow_1", bad_file_name, b"pwned") + + # Guard is shared across read/delete/size, not just save. + with pytest.raises(ValueError, match=r"flow_id|path traversal"): + await storage.get_file("../secret", "secret.txt") + with pytest.raises(ValueError, match=r"flow_id|path traversal"): + await storage.delete_file("../secret", "secret.txt") + with pytest.raises(ValueError, match=r"flow_id|path traversal"): + await storage.get_file_size("../secret", "secret.txt") + + # The sentinel outside the data dir was never written to or removed. + assert sentinel.read_text() == "original" + + +def test_registering_a_different_storage_factory_overrides_default(fresh_service_manager): + """A different storage factory registered through the manager wins.""" + from lfx.services.base import Service + from lfx.services.initialize import initialize_services + + initialize_services() + + class CustomStorageService(Service): + # Same name keys it to STORAGE_SERVICE and replaces the default factory. + name = "storage_service" + + def __init__(self) -> None: + super().__init__() + self.set_ready() + + async def teardown(self) -> None: + pass + + class CustomStorageFactory(ServiceFactory): + def __init__(self) -> None: + super().__init__() + self.service_class = CustomStorageService + self.dependencies = [] + + def create(self): + return CustomStorageService() + + fresh_service_manager.register_factory(CustomStorageFactory()) + + service = fresh_service_manager.get(ServiceType.STORAGE_SERVICE) + assert isinstance(service, CustomStorageService) + assert not isinstance(service, LocalStorageService) From 8eb49d49d7024cadbc5be2d8d5d67767d8f212b5 Mon Sep 17 00:00:00 2001 From: Debojit Kaushik Date: Tue, 23 Jun 2026 01:34:52 -0400 Subject: [PATCH 2/4] Added in-memory service for bare LFX and service registry for memory service switch based on evnironment. --- .../base/langflow/services/memory/__init__.py | 6 + .../base/langflow/services/memory/factory.py | 21 ++ .../base/langflow/services/memory/service.py | 110 +++++++++ src/backend/base/langflow/services/schema.py | 1 + src/backend/base/langflow/services/utils.py | 2 + .../services/test_memory_service_dispatch.py | 105 +++++++++ src/lfx/src/lfx/memory/__init__.py | 81 ++++--- src/lfx/src/lfx/services/deps.py | 8 + src/lfx/src/lfx/services/initialize.py | 11 +- src/lfx/src/lfx/services/memory/__init__.py | 7 + src/lfx/src/lfx/services/memory/base.py | 143 ++++++++++++ src/lfx/src/lfx/services/memory/factory.py | 37 +++ src/lfx/src/lfx/services/memory/service.py | 165 +++++++++++++ src/lfx/src/lfx/services/schema.py | 1 + src/lfx/tests/unit/memory/test_memory.py | 22 +- .../tests/unit/memory/test_memory_dispatch.py | 100 ++++---- .../unit/services/test_memory_service.py | 219 ++++++++++++++++++ 17 files changed, 957 insertions(+), 82 deletions(-) create mode 100644 src/backend/base/langflow/services/memory/__init__.py create mode 100644 src/backend/base/langflow/services/memory/factory.py create mode 100644 src/backend/base/langflow/services/memory/service.py create mode 100644 src/backend/tests/unit/services/test_memory_service_dispatch.py create mode 100644 src/lfx/src/lfx/services/memory/__init__.py create mode 100644 src/lfx/src/lfx/services/memory/base.py create mode 100644 src/lfx/src/lfx/services/memory/factory.py create mode 100644 src/lfx/src/lfx/services/memory/service.py create mode 100644 src/lfx/tests/unit/services/test_memory_service.py diff --git a/src/backend/base/langflow/services/memory/__init__.py b/src/backend/base/langflow/services/memory/__init__.py new file mode 100644 index 000000000000..f21c4fcb28e6 --- /dev/null +++ b/src/backend/base/langflow/services/memory/__init__.py @@ -0,0 +1,6 @@ +"""Langflow's DB-backed chat-message memory service.""" + +from langflow.services.memory.factory import MemoryServiceFactory +from langflow.services.memory.service import LangflowMemoryService + +__all__ = ["LangflowMemoryService", "MemoryServiceFactory"] diff --git a/src/backend/base/langflow/services/memory/factory.py b/src/backend/base/langflow/services/memory/factory.py new file mode 100644 index 000000000000..4084ef295c03 --- /dev/null +++ b/src/backend/base/langflow/services/memory/factory.py @@ -0,0 +1,21 @@ +"""Factory for langflow's DB-backed memory service.""" + +from langflow.services.factory import ServiceFactory +from langflow.services.memory.service import LangflowMemoryService + + +class MemoryServiceFactory(ServiceFactory): + """Registers ``LangflowMemoryService`` over lfx's in-memory default. + + No dependencies: the service resolves its backend lazily on first use from the + registered database service, so it must not force the DB service to be created + eagerly at factory time. ``create`` deliberately carries no parameter or return + annotations — ``infer_service_types`` evaluates them against the service-class + namespace, where ``LangflowMemoryService`` is not present. + """ + + def __init__(self) -> None: + super().__init__(LangflowMemoryService) + + def create(self): + return LangflowMemoryService() diff --git a/src/backend/base/langflow/services/memory/service.py b/src/backend/base/langflow/services/memory/service.py new file mode 100644 index 000000000000..bf30f7d195eb --- /dev/null +++ b/src/backend/base/langflow/services/memory/service.py @@ -0,0 +1,110 @@ +"""Langflow's MEMORY_SERVICE: real DB-backed memory, with an in-memory fallback. + +The backend is resolved **once, on first use**, from the registered database +service: a real (non-noop) DB routes to ``langflow.memory`` (the MessageTable +implementation), while a ``NoopDatabaseService`` falls back to lfx's round-tripping +in-memory store. Resolving on first use — rather than at construction — defers the +decision past service registration (the database service is registered at langflow +startup, before any flow runs and thus before the first memory operation), so the +service never locks in the wrong backend. + +This intentionally does *not* call ``has_langflow_db_backend()``: inside langflow, +langflow is always importable, so the only honest question is "is a real DB wired", +answered directly by inspecting the database service. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from lfx.services.memory.base import MemoryService +from lfx.services.memory.service import InMemoryMemoryService + +if TYPE_CHECKING: + from uuid import UUID + + from lfx.schema.message import Message + + +class LangflowMemoryService(MemoryService): + """Memory service that delegates to the DB-backed implementation when available.""" + + name = "memory_service" + + def __init__(self) -> None: + super().__init__() + self._backend: object | None = None + self.set_ready() + + def _backend_impl(self): + """Resolve and memoize the concrete backend on first use.""" + if self._backend is None: + from lfx.services.database.service import NoopDatabaseService + from lfx.services.deps import get_db_service + + if isinstance(get_db_service(), NoopDatabaseService): + self._backend = InMemoryMemoryService() + else: + from langflow import memory as langflow_memory + + self._backend = langflow_memory + return self._backend + + async def astore_message( + self, + message: Message, + flow_id: str | UUID | None = None, + run_id: str | UUID | None = None, + ) -> list[Message]: + return await self._backend_impl().astore_message(message, flow_id=flow_id, run_id=run_id) + + async def aget_messages( + self, + sender: str | None = None, + sender_name: str | None = None, + session_id: str | UUID | None = None, + context_id: str | UUID | None = None, + order_by: str | None = "timestamp", + order: str | None = "DESC", + flow_id: UUID | None = None, + limit: int | None = None, + ) -> list[Message]: + return await self._backend_impl().aget_messages( + sender=sender, + sender_name=sender_name, + session_id=session_id, + context_id=context_id, + order_by=order_by, + order=order, + flow_id=flow_id, + limit=limit, + ) + + async def aupdate_messages(self, messages: Message | list[Message]) -> list[Message]: + return await self._backend_impl().aupdate_messages(messages) + + async def adelete_messages(self, session_id: str | None = None, context_id: str | None = None) -> None: + return await self._backend_impl().adelete_messages(session_id=session_id, context_id=context_id) + + async def adelete_message(self, id_: str) -> None: + backend = self._backend_impl() + # langflow.memory exposes ``delete_message``; InMemoryMemoryService exposes + # ``adelete_message``. + if isinstance(backend, InMemoryMemoryService): + return await backend.adelete_message(id_) + return await backend.delete_message(id_) + + async def aadd_messages(self, messages: Message | list[Message]) -> list[Message]: + return await self._backend_impl().aadd_messages(messages) + + async def aadd_messagetables(self, messages: Message | list[Message]) -> list[Message]: + backend = self._backend_impl() + # langflow.memory.aadd_messagetables has a different (session-taking) + # signature; aadd_messages is the equivalent public batch entrypoint. + if isinstance(backend, InMemoryMemoryService): + return await backend.aadd_messagetables(messages) + return await backend.aadd_messages(messages) + + async def teardown(self) -> None: + if isinstance(self._backend, InMemoryMemoryService): + await self._backend.teardown() diff --git a/src/backend/base/langflow/services/schema.py b/src/backend/base/langflow/services/schema.py index f032a1454a17..15f66f12f1f0 100644 --- a/src/backend/base/langflow/services/schema.py +++ b/src/backend/base/langflow/services/schema.py @@ -23,5 +23,6 @@ class ServiceType(str, Enum): MCP_COMPOSER_SERVICE = "mcp_composer_service" JOB_SERVICE = "jobs_service" FLOW_EVENTS_SERVICE = "flow_events_service" + MEMORY_SERVICE = "memory_service" MEMORY_BASE_SERVICE = "memory_base_service" TELEMETRY_WRITER_SERVICE = "telemetry_writer_service" diff --git a/src/backend/base/langflow/services/utils.py b/src/backend/base/langflow/services/utils.py index daf0091507a0..65db9bbadd6c 100644 --- a/src/backend/base/langflow/services/utils.py +++ b/src/backend/base/langflow/services/utils.py @@ -576,6 +576,7 @@ def register_all_service_factories() -> None: from langflow.services.chat import factory as chat_factory from langflow.services.database import factory as database_factory from langflow.services.job_queue import factory as job_queue_factory + from langflow.services.memory import factory as memory_factory from langflow.services.session import factory as session_factory from langflow.services.shared_component_cache import factory as shared_component_cache_factory from langflow.services.state import factory as state_factory @@ -596,6 +597,7 @@ def register_all_service_factories() -> None: service_manager.register_factory(session_factory.SessionServiceFactory()) service_manager.register_factory(storage_factory.StorageServiceFactory()) service_manager.register_factory(variable_factory.VariableServiceFactory()) + service_manager.register_factory(memory_factory.MemoryServiceFactory()) service_manager.register_factory(telemetry_factory.TelemetryServiceFactory()) service_manager.register_factory(tracing_factory.TracingServiceFactory()) service_manager.register_factory(transaction_factory.TransactionServiceFactory()) diff --git a/src/backend/tests/unit/services/test_memory_service_dispatch.py b/src/backend/tests/unit/services/test_memory_service_dispatch.py new file mode 100644 index 000000000000..ce4143e2aeaf --- /dev/null +++ b/src/backend/tests/unit/services/test_memory_service_dispatch.py @@ -0,0 +1,105 @@ +"""Tests for langflow's MEMORY_SERVICE backend resolution. + +LangflowMemoryService resolves its backend once, on first use, from the registered +database service: a real DB routes to ``langflow.memory`` (MessageTable), while a +NoopDatabaseService falls back to lfx's round-tripping in-memory store (no silent +no-op inserts). The decision is cached, not re-evaluated per call. +""" + +import pytest +from langflow.services.memory.service import LangflowMemoryService +from lfx.schema.message import Message +from lfx.services.database.service import NoopDatabaseService +from lfx.services.memory.service import InMemoryMemoryService + + +def _msg(text, session_id="s1"): + return Message(text=text, sender="AI", sender_name="Bot", session_id=session_id) + + +@pytest.mark.asyncio +async def test_noop_db_resolves_in_memory_not_silent_noop(monkeypatch): + """With a NoopDatabaseService, store/read round-trip via the in-memory fallback.""" + monkeypatch.setattr("lfx.services.deps.get_db_service", lambda: NoopDatabaseService()) + + # Guard: if the fallback were wrong and it hit langflow.memory, fail loudly. + import langflow.memory as langflow_memory + + def _boom(*_args, **_kwargs): + msg = "langflow.memory must not be used under a NoopDatabaseService" + raise AssertionError(msg) + + monkeypatch.setattr(langflow_memory, "astore_message", _boom) + + svc = LangflowMemoryService() + await svc.astore_message(_msg("hello", session_id="noop")) + got = await svc.aget_messages(session_id="noop") + + assert isinstance(svc._backend_impl(), InMemoryMemoryService) + assert [m.text for m in got] == ["hello"] + + +@pytest.mark.asyncio +async def test_real_db_resolves_langflow_memory(monkeypatch): + """With a real (non-noop) DB, store/read route to langflow.memory.""" + + class _FakeRealDbService: + """Non-noop stand-in.""" + + monkeypatch.setattr("lfx.services.deps.get_db_service", lambda: _FakeRealDbService()) + + import langflow.memory as langflow_memory + + calls = {} + + async def _fake_store(message, flow_id=None, run_id=None): # noqa: ARG001 + calls["store"] = message + return [message] + + async def _fake_get(**kwargs): + calls["get"] = kwargs + return ["sentinel"] + + monkeypatch.setattr(langflow_memory, "astore_message", _fake_store) + monkeypatch.setattr(langflow_memory, "aget_messages", _fake_get) + + svc = LangflowMemoryService() + msg = _msg("hi", session_id="real") + await svc.astore_message(msg) + got = await svc.aget_messages(session_id="real") + + assert svc._backend_impl() is langflow_memory + assert calls["store"] is msg + assert got == ["sentinel"] + + +@pytest.mark.asyncio +async def test_backend_resolved_once_and_cached(monkeypatch): + """Once resolved, flipping the DB state does not change the backend.""" + monkeypatch.setattr("lfx.services.deps.get_db_service", lambda: NoopDatabaseService()) + + svc = LangflowMemoryService() + # First use under noop DB -> in-memory. + await svc.astore_message(_msg("first", session_id="cache")) + assert isinstance(svc._backend_impl(), InMemoryMemoryService) + + # Now a real DB appears; the cached backend must not switch. + class _FakeRealDbService: + pass + + monkeypatch.setattr("lfx.services.deps.get_db_service", lambda: _FakeRealDbService()) + assert isinstance(svc._backend_impl(), InMemoryMemoryService) + + # And it still round-trips through the same in-memory backend. + got = await svc.aget_messages(session_id="cache") + assert [m.text for m in got] == ["first"] + + +def test_get_memory_service_returns_langflow_service(): + """In a langflow process the registered MEMORY_SERVICE is LangflowMemoryService.""" + from langflow.services.utils import register_all_service_factories + from lfx.services.deps import get_memory_service + + register_all_service_factories() + svc = get_memory_service() + assert isinstance(svc, LangflowMemoryService) diff --git a/src/lfx/src/lfx/memory/__init__.py b/src/lfx/src/lfx/memory/__init__.py index 2ea2151a3dd5..0f5882ed6b47 100644 --- a/src/lfx/src/lfx/memory/__init__.py +++ b/src/lfx/src/lfx/memory/__init__.py @@ -1,63 +1,74 @@ -"""Memory management for lfx with dynamic dispatch. - -Routes memory operations to either the full langflow implementation (when -langflow is installed AND a real database service is registered) or the lfx -stub implementation (standalone / noop DB). - -Dispatch is evaluated at call time, not import time, because the database -service is typically registered *after* this module is first imported (e.g., -from Component class definitions loaded before graph setup). An import-time -decision can't distinguish "langflow is importable" from "a real DB is wired", -and picking the langflow backend with a NoopDatabaseService yields silent -no-op inserts followed by spurious "Message with id X not found" errors on -update. +"""Memory management for lfx, routed through the pluggable MEMORY_SERVICE. + +Every public function resolves the registered memory service via +``get_memory_service()`` *at call time*. The question this layer asks is simply +"is a memory backend registered" — not "is langflow importable and is its DB +non-noop". Which concrete backend the registered service uses (in-memory vs a +real DB) is decided by the MEMORY_SERVICE factory/service layer, not here. + +Resolution happens per call (rather than binding at import) because the service +manager — and the database service that a DB-backed backend depends on — is +typically registered *after* this module is first imported (e.g. from Component +class definitions loaded before graph setup). The call-time lookup is a cheap +manager hit that returns the already-cached service. """ from __future__ import annotations -from typing import Any +from typing import TYPE_CHECKING, Any -from lfx.utils.langflow_utils import has_langflow_db_backend +from lfx.services.deps import get_memory_service +if TYPE_CHECKING: + from lfx.services.memory.base import MemoryService -def _impl(): - if has_langflow_db_backend(): - from langflow import memory as impl - else: - from lfx.memory import stubs as impl - return impl +def _impl() -> MemoryService: + """Resolve the registered memory service. -def aadd_messages(*args: Any, **kwargs: Any): - return _impl().aadd_messages(*args, **kwargs) + The MEMORY_SERVICE factory is always registered (auto-discovered from the + ``ServiceType`` enum and registered explicitly in ``lfx.services.initialize``), + so this normally returns a real service. We guard against ``None`` only to + fail loudly on a genuinely broken service manager rather than silently route + memory into a throwaway store. + """ + service = get_memory_service() + if service is None: # pragma: no cover - the factory is always registered + msg = "No memory service is registered; cannot perform memory operations." + raise RuntimeError(msg) + return service -def aadd_messagetables(*args: Any, **kwargs: Any): - return _impl().aadd_messagetables(*args, **kwargs) +async def aadd_messages(*args: Any, **kwargs: Any): + return await _impl().aadd_messages(*args, **kwargs) + + +async def aadd_messagetables(*args: Any, **kwargs: Any): + return await _impl().aadd_messagetables(*args, **kwargs) def add_messages(*args: Any, **kwargs: Any): return _impl().add_messages(*args, **kwargs) -def adelete_messages(*args: Any, **kwargs: Any): - return _impl().adelete_messages(*args, **kwargs) +async def adelete_messages(*args: Any, **kwargs: Any): + return await _impl().adelete_messages(*args, **kwargs) -def aget_messages(*args: Any, **kwargs: Any): - return _impl().aget_messages(*args, **kwargs) +async def aget_messages(*args: Any, **kwargs: Any): + return await _impl().aget_messages(*args, **kwargs) -def astore_message(*args: Any, **kwargs: Any): - return _impl().astore_message(*args, **kwargs) +async def astore_message(*args: Any, **kwargs: Any): + return await _impl().astore_message(*args, **kwargs) -def aupdate_messages(*args: Any, **kwargs: Any): - return _impl().aupdate_messages(*args, **kwargs) +async def aupdate_messages(*args: Any, **kwargs: Any): + return await _impl().aupdate_messages(*args, **kwargs) -def delete_message(*args: Any, **kwargs: Any): - return _impl().delete_message(*args, **kwargs) +async def delete_message(*args: Any, **kwargs: Any): + return await _impl().adelete_message(*args, **kwargs) def delete_messages(*args: Any, **kwargs: Any): diff --git a/src/lfx/src/lfx/services/deps.py b/src/lfx/src/lfx/services/deps.py index 25a360190466..7705732dccae 100644 --- a/src/lfx/src/lfx/services/deps.py +++ b/src/lfx/src/lfx/services/deps.py @@ -31,6 +31,7 @@ TransactionServiceProtocol, VariableServiceProtocol, ) + from lfx.services.memory.base import MemoryService def get_service(service_type: ServiceType, default=None): @@ -106,6 +107,13 @@ def get_variable_service() -> VariableServiceProtocol | None: return get_service(ServiceType.VARIABLE_SERVICE) +def get_memory_service() -> MemoryService | None: + """Retrieves the chat-message memory service instance.""" + from lfx.services.schema import ServiceType + + return get_service(ServiceType.MEMORY_SERVICE) + + def get_shared_component_cache_service() -> CacheServiceProtocol | None: """Retrieves the shared component cache service instance.""" from lfx.services.shared_component_cache.factory import SharedComponentCacheServiceFactory diff --git a/src/lfx/src/lfx/services/initialize.py b/src/lfx/src/lfx/services/initialize.py index a088ca10804e..905ba8a26d41 100644 --- a/src/lfx/src/lfx/services/initialize.py +++ b/src/lfx/src/lfx/services/initialize.py @@ -1,5 +1,6 @@ """Initialize services for lfx package.""" +from lfx.services.memory.factory import MemoryServiceFactory from lfx.services.settings.factory import SettingsServiceFactory from lfx.services.storage.factory import StorageServiceFactory from lfx.services.variable.factory import VariableServiceFactory @@ -12,13 +13,15 @@ def initialize_services(): service_manager = get_service_manager() # Register the lean no-deps defaults. Settings is the only hard requirement; - # storage and variable are registered here so file-backed and variable-using - # components have a real service in bare lfx instead of None. These go into - # the factory tier, so a heavier backend (e.g. langflow) can override either - # through the same manager (config/decorator registrations take precedence). + # storage, variable, and memory are registered here so file-backed, + # variable-using, and chat-memory components have a real service in bare lfx + # instead of None. These go into the factory tier, so a heavier backend (e.g. + # langflow) can override any of them through the same manager (config/decorator + # registrations take precedence). service_manager.register_factory(SettingsServiceFactory()) service_manager.register_factory(StorageServiceFactory()) service_manager.register_factory(VariableServiceFactory()) + service_manager.register_factory(MemoryServiceFactory()) # Note: auth and authorization self-register at import time via the # @register_service decorator. Storage and variable do NOT self-register on diff --git a/src/lfx/src/lfx/services/memory/__init__.py b/src/lfx/src/lfx/services/memory/__init__.py new file mode 100644 index 000000000000..a9c2a791c736 --- /dev/null +++ b/src/lfx/src/lfx/services/memory/__init__.py @@ -0,0 +1,7 @@ +"""Pluggable chat-message memory service for lfx.""" + +from lfx.services.memory.base import MemoryService +from lfx.services.memory.factory import MemoryServiceFactory +from lfx.services.memory.service import InMemoryMemoryService + +__all__ = ["InMemoryMemoryService", "MemoryService", "MemoryServiceFactory"] diff --git a/src/lfx/src/lfx/services/memory/base.py b/src/lfx/src/lfx/services/memory/base.py new file mode 100644 index 000000000000..2f87f71493e3 --- /dev/null +++ b/src/lfx/src/lfx/services/memory/base.py @@ -0,0 +1,143 @@ +"""Base class for the pluggable chat-message memory service. + +The memory service models the store/read/update/delete surface that the +``lfx.memory`` module exposes for chat ``Message`` objects. Backends implement a +small set of async *primitives*; the batch helpers and the deprecated sync +wrappers are concrete on this base class so every backend inherits them (this is +why those wrappers do not need to live in ``lfx.memory.__init__``). + +This base class is also the override seam for richer backends: langflow swaps in +a DB-backed implementation, and a future ``DatabaseBackedMemoryService`` for LFX +executors (Postgres) registers through the same service manager. +""" + +from __future__ import annotations + +from abc import abstractmethod +from typing import TYPE_CHECKING + +from lfx.services.base import Service +from lfx.utils.async_helpers import run_until_complete + +if TYPE_CHECKING: + from uuid import UUID + + from lfx.schema.message import Message + + +class MemoryService(Service): + """Abstract base class for chat-message memory backends. + + Abstract primitives (each backend implements): + - ``astore_message`` — persist a single message. + - ``aget_messages`` — query/filter messages. + - ``aupdate_messages`` — update existing messages. + - ``adelete_messages`` — delete by session/context id. + - ``adelete_message`` — delete a single message by id. + + Concrete derived/convenience methods (inherited; may be overridden): + - ``aadd_messages`` / ``aadd_messagetables`` — batch over ``astore_message``. + - ``store_message`` / ``get_messages`` / ``delete_messages`` / ``add_messages`` + — deprecated sync wrappers via ``run_until_complete``. + """ + + name = "memory_service" + + # --- Abstract primitives ------------------------------------------------- + + @abstractmethod + async def astore_message( + self, + message: Message, + flow_id: str | UUID | None = None, + run_id: str | UUID | None = None, + ) -> list[Message]: + """Store a single message and return it as a one-element list.""" + + @abstractmethod + async def aget_messages( + self, + sender: str | None = None, + sender_name: str | None = None, + session_id: str | UUID | None = None, + context_id: str | UUID | None = None, + order_by: str | None = "timestamp", + order: str | None = "DESC", + flow_id: UUID | None = None, + limit: int | None = None, + ) -> list[Message]: + """Retrieve messages matching the provided filters.""" + + @abstractmethod + async def aupdate_messages(self, messages: Message | list[Message]) -> list[Message]: + """Update stored messages and return the updated list.""" + + @abstractmethod + async def adelete_messages(self, session_id: str | None = None, context_id: str | None = None) -> None: + """Delete messages by session id or context id.""" + + @abstractmethod + async def adelete_message(self, id_: str) -> None: + """Delete a single message by id.""" + + # --- Concrete derived methods ------------------------------------------- + + async def aadd_messages(self, messages: Message | list[Message]) -> list[Message]: + """Add one or more messages by delegating to ``astore_message``.""" + if not isinstance(messages, list): + messages = [messages] + result: list[Message] = [] + for message in messages: + result.extend(await self.astore_message(message)) + return result + + async def aadd_messagetables(self, messages: Message | list[Message]) -> list[Message]: + """Alias for ``aadd_messages`` kept for backwards compatibility.""" + return await self.aadd_messages(messages) + + # --- Deprecated sync wrappers ------------------------------------------- + + def store_message( + self, + message: Message, + flow_id: str | UUID | None = None, + run_id: str | UUID | None = None, + ) -> list[Message]: + """DEPRECATED: use ``astore_message`` instead.""" + return run_until_complete(self.astore_message(message, flow_id=flow_id, run_id=run_id)) + + def get_messages( + self, + sender: str | None = None, + sender_name: str | None = None, + session_id: str | UUID | None = None, + context_id: str | UUID | None = None, + order_by: str | None = "timestamp", + order: str | None = "DESC", + flow_id: UUID | None = None, + limit: int | None = None, + ) -> list[Message]: + """DEPRECATED: use ``aget_messages`` instead.""" + return run_until_complete( + self.aget_messages( + sender=sender, + sender_name=sender_name, + session_id=session_id, + context_id=context_id, + order_by=order_by, + order=order, + flow_id=flow_id, + limit=limit, + ) + ) + + def delete_messages(self, session_id: str | None = None, context_id: str | None = None) -> None: + """DEPRECATED: use ``adelete_messages`` instead.""" + return run_until_complete(self.adelete_messages(session_id=session_id, context_id=context_id)) + + def add_messages(self, messages: Message | list[Message]) -> list[Message]: + """DEPRECATED: use ``aadd_messages`` instead.""" + return run_until_complete(self.aadd_messages(messages)) + + async def teardown(self) -> None: + """Teardown the memory service. Backends with state should override.""" diff --git a/src/lfx/src/lfx/services/memory/factory.py b/src/lfx/src/lfx/services/memory/factory.py new file mode 100644 index 000000000000..ddda35929ac2 --- /dev/null +++ b/src/lfx/src/lfx/services/memory/factory.py @@ -0,0 +1,37 @@ +"""Factory for the lean in-memory memory service used by bare lfx.""" + +from __future__ import annotations + +from typing_extensions import override + +from lfx.services.factory import ServiceFactory +from lfx.services.memory.service import InMemoryMemoryService + + +class MemoryServiceFactory(ServiceFactory): + """Registers the no-deps ``InMemoryMemoryService`` as the lean default. + + The backend is chosen once, at creation time, from the registered database + service: with a real (non-noop) DB a DB-backed backend is appropriate, while + bare lfx without a database gets the round-tripping in-memory store. A heavier + backend (e.g. langflow's DB-backed memory service) overrides this through the + same service manager. + """ + + def __init__(self) -> None: + super().__init__() + self.service_class = InMemoryMemoryService + self.dependencies = [] + + @override + def create(self) -> InMemoryMemoryService: + from lfx.services.database.service import NoopDatabaseService + from lfx.services.deps import get_db_service + + if isinstance(get_db_service(), NoopDatabaseService): + return InMemoryMemoryService() + # TODO(follow-up): return DatabaseBackedMemoryService() for LFX executors + # (Postgres) once it lands. The seam is here so a non-noop DB — including a + # PG-configured bare-lfx run — resolves to the persistent backend without + # touching the ABC or the deps getter. + return InMemoryMemoryService() diff --git a/src/lfx/src/lfx/services/memory/service.py b/src/lfx/src/lfx/services/memory/service.py new file mode 100644 index 000000000000..9fcbdef90fdc --- /dev/null +++ b/src/lfx/src/lfx/services/memory/service.py @@ -0,0 +1,165 @@ +"""In-memory chat-message memory backend — the no-deps lean default for bare lfx. + +Stores ``Message`` objects in a process-local dict so that store/read genuinely +round-trips without any database. This is the default registered by +``lfx.services.initialize`` and is also used as the fallback when langflow runs +with a ``NoopDatabaseService`` (no silent no-op inserts). +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING +from uuid import UUID + +from lfx.log.logger import logger +from lfx.services.memory.base import MemoryService + +if TYPE_CHECKING: + from lfx.schema.message import Message + + +class InMemoryMemoryService(MemoryService): + """Round-tripping in-memory memory backend (no database, no dependencies).""" + + name = "memory_service" + + def __init__(self) -> None: + super().__init__() + self._messages: dict[str, Message] = {} + self.set_ready() + logger.debug("In-memory memory service initialized") + + async def astore_message( + self, + message: Message, + flow_id: str | UUID | None = None, + run_id: str | UUID | None = None, + ) -> list[Message]: + """Store a single message in the in-memory dict and return it.""" + if not message: + logger.warning("No message provided.") + return [] + + if not message.session_id or not message.sender or not message.sender_name: + msg = ( + f"All of session_id, sender, and sender_name must be provided. Session ID: {message.session_id}," + f" Sender: {message.sender}, Sender Name: {message.sender_name}" + ) + raise ValueError(msg) + + # Normalize flow_id/run_id. Be tolerant of non-UUID flow_ids (synthetic IDs + # from tests or string identifiers); UUID parsing only normalizes format. + if flow_id: + if isinstance(flow_id, str): + try: + flow_id = UUID(flow_id) + except ValueError: + logger.warning( + f"flow_id {flow_id!r} is not a valid UUID; preserving verbatim. " + "Downstream code that expects a UUID may surface a confusing error." + ) + message.flow_id = str(flow_id) + if run_id: + if isinstance(run_id, UUID): + run_id = str(run_id) + message.run_id = str(run_id) + + if not getattr(message, "id", None): + try: + import nanoid + + message.id = nanoid.generate() + except ImportError: + import uuid + + message.id = str(uuid.uuid4()) + + self._messages[str(message.id)] = message + logger.debug(f"Message stored with ID: {message.id}") + return [message] + + async def aget_messages( + self, + sender: str | None = None, + sender_name: str | None = None, + session_id: str | UUID | None = None, + context_id: str | UUID | None = None, + order_by: str | None = "timestamp", + order: str | None = "DESC", + flow_id: UUID | None = None, + limit: int | None = None, + ) -> list[Message]: + """Filter, sort, and limit the in-memory messages.""" + results = list(self._messages.values()) + + if sender is not None: + results = [m for m in results if m.sender == sender] + if sender_name is not None: + results = [m for m in results if m.sender_name == sender_name] + # session_id/context_id/flow_id may be UUID or str depending on caller; + # compare as strings so a UUID filter still matches a str-stored value. + if session_id is not None: + results = [m for m in results if str(getattr(m, "session_id", None)) == str(session_id)] + if context_id is not None: + results = [m for m in results if str(getattr(m, "context_id", None)) == str(context_id)] + if flow_id is not None: + results = [m for m in results if str(getattr(m, "flow_id", None)) == str(flow_id)] + + if order_by: + results.sort( + key=lambda m: getattr(m, order_by, None) or "", + reverse=(order or "DESC").upper() == "DESC", + ) + + if limit is not None: + results = results[:limit] + + logger.debug(f"Retrieved {len(results)} messages") + return results + + async def aupdate_messages(self, messages: Message | list[Message]) -> list[Message]: + """Upsert messages by id. + + Unlike langflow's DB backend we do not raise when a message id is absent + from the store — we upsert. This deliberately avoids the strict + existence check that surfaced "Message with id X not found" against a + NoopSession. + """ + if not isinstance(messages, list): + messages = [messages] + + updated: list[Message] = [] + for message in messages: + if not getattr(message, "id", None): + error_message = f"Message without ID cannot be updated: {message}" + logger.warning(error_message) + raise ValueError(error_message) + if message.flow_id and isinstance(message.flow_id, UUID): + message.flow_id = str(message.flow_id) + self._messages[str(message.id)] = message + updated.append(message) + logger.debug(f"Message updated: {message.id}") + return updated + + async def adelete_messages(self, session_id: str | None = None, context_id: str | None = None) -> None: + """Delete all messages matching the given session id or context id.""" + if not session_id and not context_id: + msg = "Either session_id or context_id must be provided to delete messages." + raise ValueError(msg) + + target = str(session_id) if session_id else str(context_id) + field = "session_id" if session_id else "context_id" + to_drop = [mid for mid, m in self._messages.items() if str(getattr(m, field, None)) == target] + for mid in to_drop: + del self._messages[mid] + logger.debug(f"Deleted {len(to_drop)} messages for {field}: {target}") + + async def adelete_message(self, id_: str) -> None: + """Delete a single message by id (no-op if absent).""" + self._messages.pop(str(id_), None) + logger.debug(f"Message deleted: {id_}") + + async def teardown(self) -> None: + """Clear the in-memory store.""" + self._messages.clear() + logger.debug("In-memory memory service teardown") diff --git a/src/lfx/src/lfx/services/schema.py b/src/lfx/src/lfx/services/schema.py index 1bba5de09364..c4d97fdd5fbb 100644 --- a/src/lfx/src/lfx/services/schema.py +++ b/src/lfx/src/lfx/services/schema.py @@ -28,3 +28,4 @@ class ServiceType(str, Enum): TELEMETRY_WRITER_SERVICE = "telemetry_writer_service" EXTENSION_EVENTS_SERVICE = "extension_events_service" EXECUTOR_SERVICE = "executor_service" + MEMORY_SERVICE = "memory_service" diff --git a/src/lfx/tests/unit/memory/test_memory.py b/src/lfx/tests/unit/memory/test_memory.py index 5750780d4819..39b58aa51dd0 100644 --- a/src/lfx/tests/unit/memory/test_memory.py +++ b/src/lfx/tests/unit/memory/test_memory.py @@ -19,6 +19,26 @@ from lfx.schema.message import Message +@pytest.fixture(autouse=True) +def _reset_memory_store(): + """Isolate tests: the default memory service is a process-global singleton. + + Memory now round-trips in-process (it is no longer a no-op stub), so without a + reset, messages stored by one test would leak into the next. + """ + from lfx.services.deps import get_memory_service + + def _clear(): + svc = get_memory_service() + store = getattr(svc, "_messages", None) + if store is not None: + store.clear() + + _clear() + yield + _clear() + + class TestMemoryFunctions: """Test cases for memory functions.""" @@ -146,7 +166,7 @@ def test_add_messages_list(self): def test_get_messages_basic(self): """Test getting messages basic functionality.""" - # Since this is a stub implementation, it should return empty list + # The store is reset per test, so nothing has been stored yet. result = get_messages() assert isinstance(result, list) assert len(result) == 0 diff --git a/src/lfx/tests/unit/memory/test_memory_dispatch.py b/src/lfx/tests/unit/memory/test_memory_dispatch.py index 7a61017e085c..155e954ce63e 100644 --- a/src/lfx/tests/unit/memory/test_memory_dispatch.py +++ b/src/lfx/tests/unit/memory/test_memory_dispatch.py @@ -1,11 +1,14 @@ -"""Regression tests for lfx.memory runtime dispatch. - -Original bug: when langflow was installed alongside lfx but `lfx run` had -only a NoopDatabaseService registered, `lfx.memory` bound at import time to -`langflow.memory` (because the `langflow` package was importable). The -langflow-backed `aupdate_messages` then called `session.get(...)` on a -NoopSession, which always returns `None`, raising spurious -"Message with id X not found" errors mid-stream. +"""Tests for lfx.memory dispatch through the pluggable MEMORY_SERVICE. + +Original bug: when langflow was installed alongside lfx but `lfx run` had only a +NoopDatabaseService registered, `lfx.memory` routed to `langflow.memory` simply +because the `langflow` package was importable, and its DB-backed code then hit a +NoopSession (silent no-op inserts / "Message with id X not found" on update). + +The fix moved dispatch behind MEMORY_SERVICE: `lfx.memory` resolves the registered +memory service via `get_memory_service()` at call time and no longer asks "is +langflow importable and is its DB non-noop". The DB-vs-in-memory decision lives in +the memory service factory/service layer. These tests pin that contract. """ from __future__ import annotations @@ -22,6 +25,8 @@ class _FakeRealDbService: class TestHasLangflowDbBackend: + """The util still exists (used by the factory layer); it is no longer the memory gate.""" + def test_returns_false_when_langflow_not_importable(self, monkeypatch): monkeypatch.setattr("lfx.utils.langflow_utils.has_langflow_memory", lambda: False) assert has_langflow_db_backend() is False @@ -48,61 +53,72 @@ def boom(): class TestMemoryDispatch: - def test_dispatches_to_stubs_when_no_real_db(self, monkeypatch): + def test_memory_module_does_not_import_db_backend_gate(self): + """The layering fix: memory/__init__ no longer references has_langflow_db_backend.""" + import lfx.memory as memory_mod + + assert not hasattr(memory_mod, "has_langflow_db_backend") + + def test_impl_resolves_registered_memory_service(self, monkeypatch): + """_impl() routes to whatever get_memory_service() returns — at call time.""" import lfx.memory as memory_mod - from lfx.memory import stubs - monkeypatch.setattr("lfx.memory.has_langflow_db_backend", lambda: False) - assert memory_mod._impl() is stubs + sentinel = object() + monkeypatch.setattr("lfx.memory.get_memory_service", lambda: sentinel) + assert memory_mod._impl() is sentinel - def test_dispatches_to_langflow_when_real_db(self, monkeypatch): - pytest.importorskip("langflow.memory") - import langflow.memory as langflow_memory + def test_impl_is_evaluated_per_call(self, monkeypatch): + """Dispatch reads the registry each call, not a value cached at import.""" import lfx.memory as memory_mod - monkeypatch.setattr("lfx.memory.has_langflow_db_backend", lambda: True) - assert memory_mod._impl() is langflow_memory + state = {"svc": object()} + monkeypatch.setattr("lfx.memory.get_memory_service", lambda: state["svc"]) - def test_dispatch_is_evaluated_per_call(self, monkeypatch): - """Dispatch must read the backend state each call, not cache at import. + first = memory_mod._impl() + assert first is state["svc"] - The database service is often registered *after* lfx.memory is imported - (components load first, services register during graph setup), so - memoizing the dispatcher would bind to whatever state existed at - component-module load time. - """ + state["svc"] = object() + assert memory_mod._impl() is state["svc"] + assert memory_mod._impl() is not first + + def test_public_functions_route_through_service(self, monkeypatch): + """Public proxies forward to the resolved service's matching method.""" import lfx.memory as memory_mod - from lfx.memory import stubs - state = {"real": False} - monkeypatch.setattr("lfx.memory.has_langflow_db_backend", lambda: state["real"]) + calls = {} + + class _Recorder: + async def astore_message(self, *args, **kwargs): + calls["astore_message"] = (args, kwargs) + return ["ok"] + + async def adelete_message(self, *args, **kwargs): + calls["adelete_message"] = (args, kwargs) - assert memory_mod._impl() is stubs - state["real"] = True - pytest.importorskip("langflow.memory") - import langflow.memory as langflow_memory + monkeypatch.setattr("lfx.memory.get_memory_service", lambda: _Recorder()) - assert memory_mod._impl() is langflow_memory + import asyncio + + assert asyncio.run(memory_mod.astore_message("m", flow_id="f")) == ["ok"] + assert calls["astore_message"] == (("m",), {"flow_id": "f"}) + + # delete_message proxies to the service's adelete_message primitive. + asyncio.run(memory_mod.delete_message("the-id")) + assert calls["adelete_message"] == (("the-id",), {}) class TestAupdateMessagesRegression: - """Direct regression for the original 'Message with id X not found' crash.""" + """The original 'Message with id X not found' crash must not return.""" @pytest.mark.asyncio - async def test_aupdate_messages_does_not_raise_against_noop_session(self, monkeypatch): - """Regression: route to stubs (no-op) instead of raising via langflow.memory. - - With langflow importable but only a NoopDatabaseService registered, - aupdate_messages must route to stubs and succeed silently rather than - trigger langflow.memory's strict existence check against NoopSession. - """ + async def test_aupdate_messages_does_not_raise_against_noop_db(self, monkeypatch): + """With only a NoopDatabaseService, update upserts in-memory and succeeds.""" try: from langflow.schema.message import Message except ImportError: from lfx.schema.message import Message - # Force the noop-DB branch even if a real DB happens to be registered in - # this test environment. + # Force the noop-DB branch even if a real DB is registered in this env. monkeypatch.setattr("lfx.services.deps.get_db_service", lambda: NoopDatabaseService()) from lfx.memory import aupdate_messages diff --git a/src/lfx/tests/unit/services/test_memory_service.py b/src/lfx/tests/unit/services/test_memory_service.py new file mode 100644 index 000000000000..028f02b11708 --- /dev/null +++ b/src/lfx/tests/unit/services/test_memory_service.py @@ -0,0 +1,219 @@ +"""Tests for the pluggable MEMORY_SERVICE and its lean in-memory default. + +Bare lfx previously had no round-tripping chat memory: the stub wrote through a +NoopDatabaseService and reads returned ``[]``. These tests verify the in-memory +default genuinely round-trips, supports filtering/sorting/limiting, exposes the +inherited batch/sync helpers, and is overridable through the service manager. +""" + +import asyncio + +import lfx.services.manager as manager_mod +import pytest +from lfx.schema.message import Message +from lfx.services.factory import ServiceFactory +from lfx.services.manager import ServiceManager +from lfx.services.memory.base import MemoryService +from lfx.services.memory.factory import MemoryServiceFactory +from lfx.services.memory.service import InMemoryMemoryService +from lfx.services.schema import ServiceType + + +@pytest.fixture +def fresh_service_manager(monkeypatch): + """Swap the global service manager for a fresh one (auto-restored).""" + new_manager = ServiceManager() + monkeypatch.setattr(manager_mod, "_service_manager", new_manager) + yield new_manager + asyncio.run(new_manager.teardown()) + + +def _msg(text, sender="AI", sender_name="Bot", session_id="s1", **kwargs): + return Message(text=text, sender=sender, sender_name=sender_name, session_id=session_id, **kwargs) + + +def test_factory_metadata(): + """The memory default is no-deps and (under a noop DB) yields the in-memory backend.""" + factory = MemoryServiceFactory() + assert factory.service_class is InMemoryMemoryService + assert factory.dependencies == [] + # With no real DB registered, create() resolves to the in-memory backend. + assert isinstance(factory.create(), InMemoryMemoryService) + + +def test_get_memory_service_is_in_memory_default_and_cached(fresh_service_manager): + """In a process without langflow, the deps helper returns a cached in-memory service.""" + from lfx.services.deps import get_memory_service + from lfx.services.initialize import initialize_services + + initialize_services() + + svc = get_memory_service() + assert isinstance(svc, InMemoryMemoryService) + # Resolves through the (isolated) global manager and is cached. + assert fresh_service_manager.get(ServiceType.MEMORY_SERVICE) is svc + assert get_memory_service() is svc + + +@pytest.mark.asyncio +async def test_store_and_read_round_trip(fresh_service_manager): # noqa: ARG001 + """Store a message and read it back through get_memory_service() — not [].""" + from lfx.services.deps import get_memory_service + from lfx.services.initialize import initialize_services + + initialize_services() + svc = get_memory_service() + + await svc.astore_message(_msg("hello", session_id="round-trip")) + got = await svc.aget_messages(session_id="round-trip") + + assert [m.text for m in got] == ["hello"] + + +@pytest.mark.asyncio +async def test_top_level_memory_module_round_trips(fresh_service_manager): # noqa: ARG001 + """The lfx.memory module proxies route through the registered service and round-trip.""" + from lfx.memory import aget_messages, astore_message + from lfx.services.initialize import initialize_services + + initialize_services() + + await astore_message(_msg("via-module", session_id="mod")) + got = await aget_messages(session_id="mod") + + assert [m.text for m in got] == ["via-module"] + + +@pytest.mark.asyncio +async def test_filter_sort_limit(): + """aget_messages filters by sender/session, sorts by order, and applies limit.""" + svc = InMemoryMemoryService() + + await svc.astore_message(_msg("a", sender="AI", session_id="s1", timestamp="2024-01-01 00:00:00 UTC")) + await svc.astore_message(_msg("b", sender="User", session_id="s1", timestamp="2024-01-02 00:00:00 UTC")) + await svc.astore_message(_msg("c", sender="AI", session_id="s2", timestamp="2024-01-03 00:00:00 UTC")) + + # sender filter + ai = await svc.aget_messages(sender="AI") + assert {m.text for m in ai} == {"a", "c"} + + # session filter + s1 = await svc.aget_messages(session_id="s1") + assert {m.text for m in s1} == {"a", "b"} + + # order DESC (default) vs ASC by timestamp + desc = await svc.aget_messages(session_id="s1", order="DESC") + assert [m.text for m in desc] == ["b", "a"] + asc = await svc.aget_messages(session_id="s1", order="ASC") + assert [m.text for m in asc] == ["a", "b"] + + # limit + limited = await svc.aget_messages(limit=1) + assert len(limited) == 1 + + +@pytest.mark.asyncio +async def test_aadd_messages_loops_astore(): + """The inherited batch helper stores each message and returns them all.""" + svc = InMemoryMemoryService() + + result = await svc.aadd_messages([_msg("one", session_id="b"), _msg("two", session_id="b")]) + + assert {m.text for m in result} == {"one", "two"} + assert {m.text for m in await svc.aget_messages(session_id="b")} == {"one", "two"} + + +@pytest.mark.asyncio +async def test_aupdate_messages_upserts_without_not_found_raise(): + """Update upserts by id rather than raising 'not found' (NoopSession regression).""" + svc = InMemoryMemoryService() + [stored] = await svc.astore_message(_msg("orig", session_id="u")) + + stored.text = "edited" + [updated] = await svc.aupdate_messages(stored) + assert updated.text == "edited" + + # Updating a never-stored (but id-bearing) message upserts, no exception. + fresh = _msg("brand-new", session_id="u") + fresh.id = "00000000-0000-0000-0000-000000000001" + [up] = await svc.aupdate_messages(fresh) + assert up.text == "brand-new" + + +@pytest.mark.asyncio +async def test_delete_paths(): + """adelete_message and adelete_messages remove the right entries.""" + svc = InMemoryMemoryService() + await svc.astore_message(_msg("keep", session_id="x")) + [m2] = await svc.astore_message(_msg("drop", session_id="y")) + + await svc.adelete_message(m2.id) + assert {m.text for m in await svc.aget_messages()} == {"keep"} + + await svc.adelete_messages(session_id="x") + assert await svc.aget_messages() == [] + + with pytest.raises(ValueError, match="Either session_id or context_id"): + await svc.adelete_messages() + + +def test_sync_wrappers_on_base_round_trip(): + """The deprecated sync wrappers (concrete on the ABC) round-trip.""" + svc = InMemoryMemoryService() + + svc.store_message(_msg("sync", session_id="z")) + got = svc.get_messages(session_id="z") + + assert [m.text for m in got] == ["sync"] + + +def test_astore_message_requires_identity_fields(): + """Missing session_id/sender/sender_name raises (validation preserved from stubs).""" + svc = InMemoryMemoryService() + bad = Message(text="x", sender="AI", sender_name="Bot", session_id="") + with pytest.raises(ValueError, match="session_id, sender, and sender_name"): + asyncio.run(svc.astore_message(bad)) + + +def test_alternate_factory_overrides_default(fresh_service_manager): + """A different memory factory registered through the manager wins.""" + from lfx.services.initialize import initialize_services + + initialize_services() + + class CustomMemoryService(MemoryService): + name = "memory_service" + + def __init__(self) -> None: + super().__init__() + self.set_ready() + + async def astore_message(self, message, flow_id=None, run_id=None): # noqa: ARG002 + return [message] + + async def aget_messages(self, **kwargs): # noqa: ARG002 + return [] + + async def aupdate_messages(self, messages): + return messages if isinstance(messages, list) else [messages] + + async def adelete_messages(self, session_id=None, context_id=None): # noqa: ARG002 + return None + + async def adelete_message(self, id_): # noqa: ARG002 + return None + + class CustomMemoryFactory(ServiceFactory): + def __init__(self) -> None: + super().__init__() + self.service_class = CustomMemoryService + self.dependencies = [] + + def create(self): + return CustomMemoryService() + + fresh_service_manager.register_factory(CustomMemoryFactory()) + + service = fresh_service_manager.get(ServiceType.MEMORY_SERVICE) + assert isinstance(service, CustomMemoryService) + assert not isinstance(service, InMemoryMemoryService) From 69bad2f08cb3d48a76580ba6f09295e908e2f409 Mon Sep 17 00:00:00 2001 From: Debojit Kaushik Date: Mon, 13 Jul 2026 12:58:01 -0400 Subject: [PATCH 3/4] Langflow <> LFX Service Separation. --- .gitignore | 4 + .../langflow/services/database/service.py | 27 +- src/backend/base/langflow/services/factory.py | 22 +- .../base/langflow/services/memory/__init__.py | 13 +- .../base/langflow/services/memory/factory.py | 21 -- .../base/langflow/services/memory/service.py | 110 ------- src/backend/base/langflow/services/utils.py | 8 +- .../services/test_memory_service_dispatch.py | 133 +++----- src/lfx/PLUGGABLE_SERVICES.md | 113 +++++++ src/lfx/pyproject.toml | 2 + .../src/lfx/prod-services/lfx-db/dynamodb.py | 0 .../src/lfx/prod-services/lfx-db/postgres.py | 0 src/lfx/src/lfx/services/base.py | 28 +- src/lfx/src/lfx/services/capabilities.py | 117 +++++++ src/lfx/src/lfx/services/database/service.py | 37 +++ src/lfx/src/lfx/services/database/session.py | 81 +++++ src/lfx/src/lfx/services/deps.py | 47 +-- src/lfx/src/lfx/services/interfaces.py | 16 +- src/lfx/src/lfx/services/manager.py | 284 ++++++++++++++--- src/lfx/src/lfx/services/memory/base.py | 8 +- src/lfx/src/lfx/services/memory/contract.py | 148 +++++++++ src/lfx/src/lfx/services/memory/database.py | 295 ++++++++++++++++++ src/lfx/src/lfx/services/memory/factory.py | 28 +- src/lfx/src/lfx/services/memory/service.py | 14 +- src/lfx/src/lfx/services/ports.py | 43 +++ src/lfx/src/lfx/services/storage/postgres.py | 0 .../tests/unit/services/memory/__init__.py | 0 .../services/memory/test_memory_contract.py | 136 ++++++++ .../unit/services/test_service_wiring.py | 239 ++++++++++++++ uv.lock | 4 + 30 files changed, 1648 insertions(+), 330 deletions(-) delete mode 100644 src/backend/base/langflow/services/memory/factory.py delete mode 100644 src/backend/base/langflow/services/memory/service.py create mode 100644 src/lfx/src/lfx/prod-services/lfx-db/dynamodb.py create mode 100644 src/lfx/src/lfx/prod-services/lfx-db/postgres.py create mode 100644 src/lfx/src/lfx/services/capabilities.py create mode 100644 src/lfx/src/lfx/services/database/session.py create mode 100644 src/lfx/src/lfx/services/memory/contract.py create mode 100644 src/lfx/src/lfx/services/memory/database.py create mode 100644 src/lfx/src/lfx/services/ports.py create mode 100644 src/lfx/src/lfx/services/storage/postgres.py create mode 100644 src/lfx/tests/unit/services/memory/__init__.py create mode 100644 src/lfx/tests/unit/services/memory/test_memory_contract.py create mode 100644 src/lfx/tests/unit/services/test_service_wiring.py diff --git a/.gitignore b/.gitignore index c619923a0133..bc86395130b2 100644 --- a/.gitignore +++ b/.gitignore @@ -192,6 +192,10 @@ instance/ # Scrapy stuff: .scrapy +# AI Scratch work +/scratch +scratch/ + # Sphinx documentation docs/_build/ diff --git a/src/backend/base/langflow/services/database/service.py b/src/backend/base/langflow/services/database/service.py index dc2a961cb53f..edd8b651dea1 100644 --- a/src/backend/base/langflow/services/database/service.py +++ b/src/backend/base/langflow/services/database/service.py @@ -9,13 +9,14 @@ from contextlib import asynccontextmanager, contextmanager, nullcontext from datetime import datetime, timezone from pathlib import Path -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, ClassVar import anyio import sqlalchemy as sa from alembic import command, util from alembic.config import Config from lfx.log.logger import logger +from lfx.services.capabilities import Capability, Tier from lfx.services.deps import session_scope from sqlalchemy import event, inspect from sqlalchemy.dialects import sqlite as dialect_sqlite @@ -255,6 +256,30 @@ def check_sqlite_database_path(database_url: str) -> None: class DatabaseService(Service): name = "database_service" + # Tier 1 infrastructure port. A real engine persists across restarts; SHARED + # (cross-process) is only true for Postgres, so the static class-level + # guarantee is the intersection: {PERSISTENT}. (A backend that wants to + # advertise SHARED can override per deployment.) + tier: ClassVar[Tier] = Tier.INFRASTRUCTURE + capabilities: ClassVar[frozenset[Capability]] = frozenset({Capability.PERSISTENT}) + + def session_scope(self): + """Async write session scope over this service (auto-commit/rollback). + + Part of the Tier 1 database port (Option B): Tier 2 services call this on + their injected ``database_service``. Delegates to the shared helper so + semantics match the module-level ``lfx.services.deps.session_scope``. + """ + from lfx.services.database.session import session_scope_for + + return session_scope_for(self) + + def session_scope_readonly(self): + """Read-only session scope over this service (no commit/rollback).""" + from lfx.services.database.session import session_scope_readonly_for + + return session_scope_readonly_for(self) + def __init__(self, settings_service: SettingsService): self._logged_pragma = False self.settings_service = settings_service diff --git a/src/backend/base/langflow/services/factory.py b/src/backend/base/langflow/services/factory.py index 6cedc2ad19ee..6c36fc69713d 100644 --- a/src/backend/base/langflow/services/factory.py +++ b/src/backend/base/langflow/services/factory.py @@ -76,13 +76,23 @@ def import_all_services_into_a_dict(): try: service_name = ServiceType(service_type).value.replace("_service", "") - # Special handling for mcp_composer which is now in lfx module - if service_name == "mcp_composer": - module_name = f"lfx.services.{service_name}.service" - else: - module_name = f"langflow.services.{service_name}.service" + # Some services are hosted in lfx rather than langflow (e.g. memory, + # mcp_composer). Prefer langflow's module when it exists (an override), + # otherwise fall back to the lfx module. A service whose ``.service`` + # module exists in neither is simply skipped. + module = None + for module_name in ( + f"langflow.services.{service_name}.service", + f"lfx.services.{service_name}.service", + ): + try: + module = importlib.import_module(module_name) + break + except ModuleNotFoundError: + continue + if module is None: + continue - module = importlib.import_module(module_name) services.update( { name: obj diff --git a/src/backend/base/langflow/services/memory/__init__.py b/src/backend/base/langflow/services/memory/__init__.py index f21c4fcb28e6..279cb339bfe7 100644 --- a/src/backend/base/langflow/services/memory/__init__.py +++ b/src/backend/base/langflow/services/memory/__init__.py @@ -1,6 +1,11 @@ -"""Langflow's DB-backed chat-message memory service.""" +"""Langflow chat-message memory: the converged lfx Tier 2 service. -from langflow.services.memory.factory import MemoryServiceFactory -from langflow.services.memory.service import LangflowMemoryService +Langflow no longer defines its own memory service class. It *selects* lfx's +``DatabaseMemoryService`` and wires it over langflow's Tier 1 ``DatabaseService`` +(done in ``langflow.services.utils.register_all_service_factories``). This module +re-exports the lfx class for any code that imported the langflow path. +""" -__all__ = ["LangflowMemoryService", "MemoryServiceFactory"] +from lfx.services.memory.database import DatabaseMemoryService + +__all__ = ["DatabaseMemoryService"] diff --git a/src/backend/base/langflow/services/memory/factory.py b/src/backend/base/langflow/services/memory/factory.py deleted file mode 100644 index 4084ef295c03..000000000000 --- a/src/backend/base/langflow/services/memory/factory.py +++ /dev/null @@ -1,21 +0,0 @@ -"""Factory for langflow's DB-backed memory service.""" - -from langflow.services.factory import ServiceFactory -from langflow.services.memory.service import LangflowMemoryService - - -class MemoryServiceFactory(ServiceFactory): - """Registers ``LangflowMemoryService`` over lfx's in-memory default. - - No dependencies: the service resolves its backend lazily on first use from the - registered database service, so it must not force the DB service to be created - eagerly at factory time. ``create`` deliberately carries no parameter or return - annotations — ``infer_service_types`` evaluates them against the service-class - namespace, where ``LangflowMemoryService`` is not present. - """ - - def __init__(self) -> None: - super().__init__(LangflowMemoryService) - - def create(self): - return LangflowMemoryService() diff --git a/src/backend/base/langflow/services/memory/service.py b/src/backend/base/langflow/services/memory/service.py deleted file mode 100644 index bf30f7d195eb..000000000000 --- a/src/backend/base/langflow/services/memory/service.py +++ /dev/null @@ -1,110 +0,0 @@ -"""Langflow's MEMORY_SERVICE: real DB-backed memory, with an in-memory fallback. - -The backend is resolved **once, on first use**, from the registered database -service: a real (non-noop) DB routes to ``langflow.memory`` (the MessageTable -implementation), while a ``NoopDatabaseService`` falls back to lfx's round-tripping -in-memory store. Resolving on first use — rather than at construction — defers the -decision past service registration (the database service is registered at langflow -startup, before any flow runs and thus before the first memory operation), so the -service never locks in the wrong backend. - -This intentionally does *not* call ``has_langflow_db_backend()``: inside langflow, -langflow is always importable, so the only honest question is "is a real DB wired", -answered directly by inspecting the database service. -""" - -from __future__ import annotations - -from typing import TYPE_CHECKING - -from lfx.services.memory.base import MemoryService -from lfx.services.memory.service import InMemoryMemoryService - -if TYPE_CHECKING: - from uuid import UUID - - from lfx.schema.message import Message - - -class LangflowMemoryService(MemoryService): - """Memory service that delegates to the DB-backed implementation when available.""" - - name = "memory_service" - - def __init__(self) -> None: - super().__init__() - self._backend: object | None = None - self.set_ready() - - def _backend_impl(self): - """Resolve and memoize the concrete backend on first use.""" - if self._backend is None: - from lfx.services.database.service import NoopDatabaseService - from lfx.services.deps import get_db_service - - if isinstance(get_db_service(), NoopDatabaseService): - self._backend = InMemoryMemoryService() - else: - from langflow import memory as langflow_memory - - self._backend = langflow_memory - return self._backend - - async def astore_message( - self, - message: Message, - flow_id: str | UUID | None = None, - run_id: str | UUID | None = None, - ) -> list[Message]: - return await self._backend_impl().astore_message(message, flow_id=flow_id, run_id=run_id) - - async def aget_messages( - self, - sender: str | None = None, - sender_name: str | None = None, - session_id: str | UUID | None = None, - context_id: str | UUID | None = None, - order_by: str | None = "timestamp", - order: str | None = "DESC", - flow_id: UUID | None = None, - limit: int | None = None, - ) -> list[Message]: - return await self._backend_impl().aget_messages( - sender=sender, - sender_name=sender_name, - session_id=session_id, - context_id=context_id, - order_by=order_by, - order=order, - flow_id=flow_id, - limit=limit, - ) - - async def aupdate_messages(self, messages: Message | list[Message]) -> list[Message]: - return await self._backend_impl().aupdate_messages(messages) - - async def adelete_messages(self, session_id: str | None = None, context_id: str | None = None) -> None: - return await self._backend_impl().adelete_messages(session_id=session_id, context_id=context_id) - - async def adelete_message(self, id_: str) -> None: - backend = self._backend_impl() - # langflow.memory exposes ``delete_message``; InMemoryMemoryService exposes - # ``adelete_message``. - if isinstance(backend, InMemoryMemoryService): - return await backend.adelete_message(id_) - return await backend.delete_message(id_) - - async def aadd_messages(self, messages: Message | list[Message]) -> list[Message]: - return await self._backend_impl().aadd_messages(messages) - - async def aadd_messagetables(self, messages: Message | list[Message]) -> list[Message]: - backend = self._backend_impl() - # langflow.memory.aadd_messagetables has a different (session-taking) - # signature; aadd_messages is the equivalent public batch entrypoint. - if isinstance(backend, InMemoryMemoryService): - return await backend.aadd_messagetables(messages) - return await backend.aadd_messages(messages) - - async def teardown(self) -> None: - if isinstance(self._backend, InMemoryMemoryService): - await self._backend.teardown() diff --git a/src/backend/base/langflow/services/utils.py b/src/backend/base/langflow/services/utils.py index 65db9bbadd6c..d63326cd1767 100644 --- a/src/backend/base/langflow/services/utils.py +++ b/src/backend/base/langflow/services/utils.py @@ -566,6 +566,7 @@ def register_all_service_factories() -> None: service_manager = get_service_manager() from lfx.services.executor import factory as executor_factory from lfx.services.mcp_composer import factory as mcp_composer_factory + from lfx.services.memory.database import DatabaseMemoryService from lfx.services.settings import factory as settings_factory from langflow.services.auth import factory as auth_factory @@ -576,7 +577,6 @@ def register_all_service_factories() -> None: from langflow.services.chat import factory as chat_factory from langflow.services.database import factory as database_factory from langflow.services.job_queue import factory as job_queue_factory - from langflow.services.memory import factory as memory_factory from langflow.services.session import factory as session_factory from langflow.services.shared_component_cache import factory as shared_component_cache_factory from langflow.services.state import factory as state_factory @@ -597,7 +597,11 @@ def register_all_service_factories() -> None: service_manager.register_factory(session_factory.SessionServiceFactory()) service_manager.register_factory(storage_factory.StorageServiceFactory()) service_manager.register_factory(variable_factory.VariableServiceFactory()) - service_manager.register_factory(memory_factory.MemoryServiceFactory()) + # Memory (Tier 2) converges on the lfx implementation: langflow selects the + # lfx DatabaseMemoryService rather than subclassing. Registered as a service + # class (not a factory) so the manager injects its Tier 1 database_service + # dependency (Option B). This takes precedence over lfx's InMemory default. + service_manager.register_service_class(ServiceType.MEMORY_SERVICE, DatabaseMemoryService, override=True) service_manager.register_factory(telemetry_factory.TelemetryServiceFactory()) service_manager.register_factory(tracing_factory.TracingServiceFactory()) service_manager.register_factory(transaction_factory.TransactionServiceFactory()) diff --git a/src/backend/tests/unit/services/test_memory_service_dispatch.py b/src/backend/tests/unit/services/test_memory_service_dispatch.py index ce4143e2aeaf..b2b844952fdd 100644 --- a/src/backend/tests/unit/services/test_memory_service_dispatch.py +++ b/src/backend/tests/unit/services/test_memory_service_dispatch.py @@ -1,105 +1,72 @@ -"""Tests for langflow's MEMORY_SERVICE backend resolution. +"""Tests for langflow's MEMORY_SERVICE wiring (converged on the lfx Tier 2 service). -LangflowMemoryService resolves its backend once, on first use, from the registered -database service: a real DB routes to ``langflow.memory`` (MessageTable), while a -NoopDatabaseService falls back to lfx's round-tripping in-memory store (no silent -no-op inserts). The decision is cached, not re-evaluated per call. +Langflow no longer defines its own memory service or sniffs the database backend +at call time. It *selects* lfx's ``DatabaseMemoryService`` and the service manager +injects langflow's Tier 1 ``DatabaseService`` (Option B). These tests assert the +registration/wiring; behavioral equivalence across backends is covered by the +shared contract suite in lfx (``lfx.services.memory.contract``). """ import pytest -from langflow.services.memory.service import LangflowMemoryService -from lfx.schema.message import Message -from lfx.services.database.service import NoopDatabaseService -from lfx.services.memory.service import InMemoryMemoryService +from langflow.services.memory import DatabaseMemoryService +from lfx.services.capabilities import Capability, ServiceWiringError, Tier +from lfx.services.manager import get_service_manager +from lfx.services.schema import ServiceType -def _msg(text, session_id="s1"): - return Message(text=text, sender="AI", sender_name="Bot", session_id=session_id) - - -@pytest.mark.asyncio -async def test_noop_db_resolves_in_memory_not_silent_noop(monkeypatch): - """With a NoopDatabaseService, store/read round-trip via the in-memory fallback.""" - monkeypatch.setattr("lfx.services.deps.get_db_service", lambda: NoopDatabaseService()) - - # Guard: if the fallback were wrong and it hit langflow.memory, fail loudly. - import langflow.memory as langflow_memory - - def _boom(*_args, **_kwargs): - msg = "langflow.memory must not be used under a NoopDatabaseService" - raise AssertionError(msg) - - monkeypatch.setattr(langflow_memory, "astore_message", _boom) - - svc = LangflowMemoryService() - await svc.astore_message(_msg("hello", session_id="noop")) - got = await svc.aget_messages(session_id="noop") - - assert isinstance(svc._backend_impl(), InMemoryMemoryService) - assert [m.text for m in got] == ["hello"] - - -@pytest.mark.asyncio -async def test_real_db_resolves_langflow_memory(monkeypatch): - """With a real (non-noop) DB, store/read route to langflow.memory.""" - - class _FakeRealDbService: - """Non-noop stand-in.""" - - monkeypatch.setattr("lfx.services.deps.get_db_service", lambda: _FakeRealDbService()) - - import langflow.memory as langflow_memory - - calls = {} +def test_langflow_selects_lfx_database_memory_service(): + """After registration, MEMORY_SERVICE resolves to the lfx DatabaseMemoryService class.""" + from langflow.services.utils import register_all_service_factories - async def _fake_store(message, flow_id=None, run_id=None): # noqa: ARG001 - calls["store"] = message - return [message] + register_all_service_factories() + manager = get_service_manager() - async def _fake_get(**kwargs): - calls["get"] = kwargs - return ["sentinel"] + # The registered class (no instantiation needed) is the converged lfx impl — + # langflow authors no memory subclass of its own. + assert manager.service_classes[ServiceType.MEMORY_SERVICE] is DatabaseMemoryService - monkeypatch.setattr(langflow_memory, "astore_message", _fake_store) - monkeypatch.setattr(langflow_memory, "aget_messages", _fake_get) - svc = LangflowMemoryService() - msg = _msg("hi", session_id="real") - await svc.astore_message(msg) - got = await svc.aget_messages(session_id="real") +def test_memory_is_tier2_requiring_database(): + """DatabaseMemoryService is Tier 2 and declares a dependency on the database service.""" + assert DatabaseMemoryService.tier == Tier.COMPOSED + assert Capability.PERSISTENT in DatabaseMemoryService.capabilities + required = {req.service for req in DatabaseMemoryService.requires} + assert ServiceType.DATABASE_SERVICE in required - assert svc._backend_impl() is langflow_memory - assert calls["store"] is msg - assert got == ["sentinel"] +def test_memory_requires_database_presence_only(): + """The requirement is presence, not a capability — so any DB backend satisfies it.""" + db_reqs = [req for req in DatabaseMemoryService.requires if req.service is ServiceType.DATABASE_SERVICE] + assert db_reqs, "expected a database_service requirement" + # Empty capability set => presence-only (bare lfx run over an ephemeral DB is fine). + assert all(req.capabilities == frozenset() for req in db_reqs) -@pytest.mark.asyncio -async def test_backend_resolved_once_and_cached(monkeypatch): - """Once resolved, flipping the DB state does not change the backend.""" - monkeypatch.setattr("lfx.services.deps.get_db_service", lambda: NoopDatabaseService()) - svc = LangflowMemoryService() - # First use under noop DB -> in-memory. - await svc.astore_message(_msg("first", session_id="cache")) - assert isinstance(svc._backend_impl(), InMemoryMemoryService) +def test_manifest_reports_langflow_wiring(): + """The wiring manifest reports memory (Tier 2) and database (Tier 1) after registration.""" + from langflow.services.utils import register_all_service_factories - # Now a real DB appears; the cached backend must not switch. - class _FakeRealDbService: - pass + register_all_service_factories() + manager = get_service_manager() + manifest = manager.wiring_manifest(discover=False) - monkeypatch.setattr("lfx.services.deps.get_db_service", lambda: _FakeRealDbService()) - assert isinstance(svc._backend_impl(), InMemoryMemoryService) + memory_entry = manifest[ServiceType.MEMORY_SERVICE] + assert memory_entry.impl_class == "DatabaseMemoryService" + assert memory_entry.tier == int(Tier.COMPOSED) - # And it still round-trips through the same in-memory backend. - got = await svc.aget_messages(session_id="cache") - assert [m.text for m in got] == ["first"] + db_entry = manifest[ServiceType.DATABASE_SERVICE] + assert db_entry.tier == int(Tier.INFRASTRUCTURE) + assert Capability.PERSISTENT in db_entry.capabilities -def test_get_memory_service_returns_langflow_service(): - """In a langflow process the registered MEMORY_SERVICE is LangflowMemoryService.""" +def test_validate_wiring_passes_for_langflow_stack(): + """The full langflow registration validates without raising (deps + capabilities satisfied).""" from langflow.services.utils import register_all_service_factories - from lfx.services.deps import get_memory_service register_all_service_factories() - svc = get_memory_service() - assert isinstance(svc, LangflowMemoryService) + manager = get_service_manager() + # Should not raise: memory (Tier 2) requires database (Tier 1) presence, which is registered. + try: + manager.validate_wiring(discover=False) + except ServiceWiringError as exc: # pragma: no cover - failure path + pytest.fail(f"langflow wiring failed validation: {exc}") diff --git a/src/lfx/PLUGGABLE_SERVICES.md b/src/lfx/PLUGGABLE_SERVICES.md index 3a2f26ca2fd8..b213a0a1fa6c 100644 --- a/src/lfx/PLUGGABLE_SERVICES.md +++ b/src/lfx/PLUGGABLE_SERVICES.md @@ -15,6 +15,119 @@ The pluggable services system supports **three** discovery mechanisms: 2. Decorator registration 3. Configuration files (highest priority) +## Two-Tier Services, Capabilities, and Wiring Validation + +Services are organized into two tiers, and the service manager validates the whole +dependency graph *before instantiating anything*. This is what keeps the "same engine +code, different wiring" promise safe across bare `lfx run`, `lfx serve`, a Knative +worker-plane, and langflow-with-lfx-inside. + +### Tiers + +- **Tier 1 — infrastructure** (`Tier.INFRASTRUCTURE`): integrates one external + component and speaks generic primitives (sessions, bytes, key/value, spans). + Examples: `database_service`, `cache_service`, `storage_service`, `job_queue_service`. +- **Tier 2 — composed** (`Tier.COMPOSED`): owns domain *behavior* over lfx-owned models + and exposes a facade, but delegates the actual commit/read to Tier 1 services. + Examples: `memory_service`, `variable_service`, `transaction_service`. + +Declare the tier as a class attribute: + +```python +from lfx.services.capabilities import Tier + +class MemoryService(Service): + tier = Tier.COMPOSED +``` + +**Layering invariant** (enforced): a Tier 1 service may depend only on `settings` or on +other Tier 1 services. A Tier 1 service that declares a dependency on a Tier 2 service +fails `validate_wiring()` with a `ServiceWiringError`. + +### Capabilities and `requires` + +A **capability** is a quality attribute an implementation guarantees — coarse and about +behavior, never about which class provides it: + +- `PERSISTENT` — state survives a process restart. +- `SHARED` — state is visible across processes/pods (safe for a multi-replica worker-plane). +- `QUERYABLE` — supports filtered/ordered reads, not just fire-and-forget writes. + +A Tier 2 service declares what it needs from Tier 1 via `requires`. Each `Requires` +optionally constrains the dependency's capabilities; an empty set means "must be present, +but no capability demanded": + +```python +from typing import ClassVar +from lfx.services.capabilities import Capability, Requires +from lfx.services.schema import ServiceType + +class DatabaseMemoryService(MemoryService): + capabilities: ClassVar[frozenset[Capability]] = frozenset({Capability.QUERYABLE, Capability.PERSISTENT}) + # Presence-only: chat memory works over an ephemeral DB (bare lfx run) too. + requires: ClassVar[tuple[Requires, ...]] = (Requires(ServiceType.DATABASE_SERVICE),) + + def __init__(self, database_service): # injected by the manager (Option B) + self.database_service = database_service +``` + +The manager resolves `requires` **before** `__init__`, creates the dependency in +topological order, injects it, and enforces the declared capabilities. Because +`capabilities`/`requires`/`tier` are class attributes, the manager can reason about the +entire graph without constructing anything. + +### Ports (registration validation) + +`lfx.services.ports.SERVICE_PORTS` maps a `ServiceType` to the abstract base every +implementation must subclass. Registration through **any** source (decorator, config, +entry point) rejects a class that is not a subclass of the declared port — so an +unrelated class that happens to reuse a service key cannot silently replace a built-in +service. Types absent from the map are not validated (back-compat). + +### Boot-time validation, manifest, and fingerprint + +- `get_service_manager().validate_wiring()` — eagerly walks every registered service's + `requires` edges and raises `ServiceWiringError` on the first unmet dependency, + capability shortfall, layering violation, or cycle. Call it at startup so a + misconfigured deployment fails before accepting traffic instead of mid-request. +- `wiring_manifest()` — `{ServiceType: {impl_class, package, tier, capabilities}}`, + resolved without instantiation. Good for a `/health/services` endpoint. +- `wiring_fingerprint()` — a stable hash of the per-type **capability sets** (class names + and packages are deliberately excluded). Two deployments whose services advertise the + same capabilities produce the same fingerprint even if the concrete classes differ. + This makes a builder-vs-production divergence check fire on *behavioral* difference, not + on the unavoidable fact that different packages own the classes. + +### Reference example: chat memory (Tier 2 over Tier 1) + +`lfx.services.memory` is the worked example: + +- `base.py` — the `MemoryService` port (abstract primitives + inherited helpers). +- `service.py` — `InMemoryMemoryService`, the zero-dependency default for bare `lfx run` + (`{QUERYABLE}`, `requires=()`; ephemeral but real round-tripping memory). +- `database.py` — `DatabaseMemoryService`, the Tier 2 service that requires + `database_service` and persists through it (`{QUERYABLE, PERSISTENT}`). +- `contract.py` — `MemoryServiceContract`, an importable test mixin **both** backends + must pass. Semantics live in the contract; implementations may differ only in + capabilities, never in observable behavior. + +**How langflow consumes it (the registration answer):** langflow does *not* subclass the +memory service. It selects the lfx `DatabaseMemoryService` and lets the manager inject +langflow's Tier 1 `DatabaseService`: + +```python +# langflow.services.utils.register_all_service_factories +service_manager.register_service_class( + ServiceType.MEMORY_SERVICE, DatabaseMemoryService, override=True +) +``` + +Registered as a *service class* (not a factory) so the manager performs the `requires` +injection. This takes precedence over lfx's `InMemoryMemoryService` default. Same class in +the builder and the production worker-plane; only the Tier 1 `DatabaseService` beneath it +changes. That is the target: **overriding is the exception (auth, RBAC, genuinely +different backends), not how langflow consumes lfx.** + ## Adapter Registries (Service-Scoped Plugin Registries) LFX also supports **adapter registries** -- collections of swappable implementations that share the same protocol. diff --git a/src/lfx/pyproject.toml b/src/lfx/pyproject.toml index 5dda14753a69..9957fdbdcca9 100644 --- a/src/lfx/pyproject.toml +++ b/src/lfx/pyproject.toml @@ -122,6 +122,8 @@ markers = [ [dependency-groups] dev = [ + "aiosqlite>=0.20.0", + "greenlet>=3.0.0", "asgi-lifespan>=2.1.0", "blockbuster>=1.5.25", "coverage>=7.9.2", diff --git a/src/lfx/src/lfx/prod-services/lfx-db/dynamodb.py b/src/lfx/src/lfx/prod-services/lfx-db/dynamodb.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/src/lfx/src/lfx/prod-services/lfx-db/postgres.py b/src/lfx/src/lfx/prod-services/lfx-db/postgres.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/src/lfx/src/lfx/services/base.py b/src/lfx/src/lfx/services/base.py index 835ccf737a90..85c7de1ca5ce 100644 --- a/src/lfx/src/lfx/services/base.py +++ b/src/lfx/src/lfx/services/base.py @@ -1,10 +1,36 @@ """Base service classes for lfx package.""" from abc import ABC, abstractmethod +from typing import ClassVar + +from lfx.services.capabilities import Capability, Requires, Tier class Service(ABC): - """Base service class.""" + """Base service class. + + Subclasses may declare three class attributes that the service manager reads + *without instantiating the service* to resolve and validate wiring (see + ``lfx.services.capabilities`` and ``ServiceManager.validate_wiring``): + + - ``tier`` — layering position (Tier 1 infrastructure vs Tier 2 composed). + ``None`` (the default) opts a service out of the layering check. + - ``capabilities`` — quality attributes this implementation guarantees. + Compared against dependents' ``Requires`` and hashed into the wiring + fingerprint. + - ``requires`` — declared dependencies on other service types, optionally + constrained by capability. Resolved before ``__init__`` and injected by + the manager. + + These are declarations about the *class*, so they are class attributes, not + instance state — the manager reasons about the whole graph before any + service is built. + """ + + # Wiring declarations (class-level; read pre-instantiation by the manager). + tier: ClassVar[Tier | None] = None + capabilities: ClassVar[frozenset[Capability]] = frozenset() + requires: ClassVar[tuple[Requires, ...]] = () def __init__(self): self._ready = False diff --git a/src/lfx/src/lfx/services/capabilities.py b/src/lfx/src/lfx/services/capabilities.py new file mode 100644 index 000000000000..c48d6a7f44ed --- /dev/null +++ b/src/lfx/src/lfx/services/capabilities.py @@ -0,0 +1,117 @@ +"""Service tiers, capabilities, and declared dependencies. + +This module holds the small vocabulary the service manager uses to reason about +*wiring* — which service implementations are registered and whether they can +satisfy each other — **before** any service is instantiated. + +Three concepts: + +- ``Tier`` — the layering position of a service. ``INFRASTRUCTURE`` (Tier 1) + services integrate one external component and speak generic primitives + (sessions, bytes, key/value, spans). ``COMPOSED`` (Tier 2) services own domain + behavior over lfx-owned models and delegate the actual commit/read to Tier 1 + services. The layering invariant is one-way: a Tier 1 service may only depend + on ``settings`` or other Tier 1 services; a Tier 1 that requires a Tier 2 is a + layering violation. + +- ``Capability`` — a quality attribute an implementation provides (e.g. does its + state survive a restart, is it visible across processes). Capabilities are the + unit of comparison for "does dependency X satisfy dependent Y" and for the + builder-vs-production wiring fingerprint. They are deliberately coarse: they + describe *what an implementation guarantees*, never *which class provides it*. + +- ``Requires`` — a declared dependency edge from a dependent service onto another + ``ServiceType``, optionally constrained to require certain capabilities of the + resolved implementation. + +All three are declared as **class attributes** on services (see +``lfx.services.base.Service``) so the manager can resolve and validate the whole +dependency graph without constructing anything (no DB connections, no side +effects). See ``ServiceManager.validate_wiring``. +""" + +from __future__ import annotations + +from dataclasses import dataclass, field +from enum import Enum, IntEnum +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from lfx.services.schema import ServiceType + + +class Tier(IntEnum): + """Layering position of a service. + + ``INFRASTRUCTURE`` (1) services are leaf integrations over external + components. ``COMPOSED`` (2) services compose Tier 1 services plus domain + logic. The numeric ordering encodes the allowed dependency direction: + a service may only depend on services at the same or a lower tier. + """ + + INFRASTRUCTURE = 1 + COMPOSED = 2 + + +class Capability(str, Enum): + """A quality attribute an implementation guarantees. + + Kept deliberately small and coarse. These describe behavior, not + implementation — two different classes that both persist to a shared + Postgres advertise the same ``{PERSISTENT, SHARED}`` set, and the wiring + fingerprint treats them as equivalent. + """ + + # State survives a process restart (backed by durable storage). + PERSISTENT = "persistent" + # State is visible across processes/pods (safe for a disaggregated, + # multi-replica worker-plane), not just within one process. + SHARED = "shared" + # Supports filtered/ordered reads, not just fire-and-forget writes. + QUERYABLE = "queryable" + + +@dataclass(frozen=True) +class Requires: + """A declared dependency of one service on another ``ServiceType``. + + Args: + service: The ``ServiceType`` this service depends on. + capabilities: Capabilities the *resolved* implementation of ``service`` + must provide. Empty (the default) means "any registered + implementation will do" — i.e. the dependency is required to be + *present*, but no quality attribute is demanded of it. + """ + + service: ServiceType + capabilities: frozenset[Capability] = field(default_factory=frozenset) + + +class ServiceWiringError(RuntimeError): + """Raised when the registered services cannot satisfy a declared requirement. + + Raised eagerly at ``validate_wiring()`` (boot) rather than lazily on first + service access, so a misconfigured deployment fails before it accepts + traffic instead of mid-request. Causes include: a required dependency has no + registered implementation, a dependency's implementation lacks a required + capability, a Tier 1 service declares a dependency on a Tier 2 service + (layering violation), or a dependency cycle. + """ + + +@dataclass(frozen=True) +class WiringManifestEntry: + """One resolved service in a wiring manifest. + + Records the concrete implementation chosen for a ``ServiceType`` without + instantiating it. ``capabilities`` is the load-bearing field for the + fingerprint; ``impl_class`` and ``package`` are for humans and are + deliberately excluded from the fingerprint (see + ``ServiceManager.wiring_fingerprint``). + """ + + service_type: str + impl_class: str + package: str + tier: int + capabilities: frozenset[Capability] diff --git a/src/lfx/src/lfx/services/database/service.py b/src/lfx/src/lfx/services/database/service.py index dac6991b75d5..f2ff4a4ccd4b 100644 --- a/src/lfx/src/lfx/services/database/service.py +++ b/src/lfx/src/lfx/services/database/service.py @@ -3,6 +3,14 @@ from __future__ import annotations from contextlib import asynccontextmanager +from typing import TYPE_CHECKING, ClassVar + +from lfx.services.capabilities import Capability, Tier + +if TYPE_CHECKING: + from collections.abc import AsyncGenerator + + from sqlalchemy.ext.asyncio import AsyncSession class NoopDatabaseService: @@ -10,8 +18,21 @@ class NoopDatabaseService: This provides a database service interface that always returns NoopSession, allowing lfx to work without a real database connection. + + As a Tier 1 (infrastructure) service it declares no capabilities: a + ``NoopSession`` neither persists across restarts nor shares state across + processes. A Tier 2 service that ``Requires`` the database with + ``{PERSISTENT}`` therefore fails ``validate_wiring()`` against this + implementation — which is the desired loud-at-boot behavior instead of + silent no-op writes. """ + # Tier 1 infrastructure port. NoopSession is in-process and ephemeral, so no + # capability is advertised. (The chat-memory service requires the database to + # be *present*, not PERSISTENT, so it still wires successfully over this.) + tier: ClassVar[Tier] = Tier.INFRASTRUCTURE + capabilities: ClassVar[frozenset[Capability]] = frozenset() + @asynccontextmanager async def _with_session(self): """Internal method to create a session. DO NOT USE DIRECTLY. @@ -23,3 +44,19 @@ async def _with_session(self): async with NoopSession() as session: yield session + + def session_scope(self) -> AsyncGenerator[AsyncSession, None]: + """Write session scope over this service (auto-commit/rollback). + + Part of the Tier 1 database port: Tier 2 services call this on their + injected ``database_service`` rather than reaching for a global. + """ + from lfx.services.database.session import session_scope_for + + return session_scope_for(self) + + def session_scope_readonly(self) -> AsyncGenerator[AsyncSession, None]: + """Read-only session scope over this service (no commit/rollback).""" + from lfx.services.database.session import session_scope_readonly_for + + return session_scope_readonly_for(self) diff --git a/src/lfx/src/lfx/services/database/session.py b/src/lfx/src/lfx/services/database/session.py new file mode 100644 index 000000000000..1511f20c9111 --- /dev/null +++ b/src/lfx/src/lfx/services/database/session.py @@ -0,0 +1,81 @@ +"""Shared session-scope logic, parameterized by a database service. + +Option B of the two-tier service design promotes ``session_scope`` / +``session_scope_readonly`` onto the DatabaseService *port* (they become methods a +Tier 2 service calls on its injected ``database_service``), rather than only +existing as module-level functions that reach for a global service. + +To keep exactly one copy of the commit/rollback semantics, that logic lives here +as free functions parameterized by the db service. Both the port methods +(``DatabaseService.session_scope``) and the back-compat module-level helpers in +``lfx.services.deps`` delegate here, so there is a single source of truth and no +behavioral drift between "call the method on my injected dependency" and "call +the global function". +""" + +from __future__ import annotations + +from contextlib import asynccontextmanager, suppress +from typing import TYPE_CHECKING, Any, Protocol + +from fastapi import HTTPException + +from lfx.log.logger import logger + +if TYPE_CHECKING: + from collections.abc import AsyncGenerator + + from sqlalchemy.ext.asyncio import AsyncSession + + +class _SupportsWithSession(Protocol): + """Minimal surface the scope helpers need from a database service.""" + + def _with_session(self) -> Any: ... + + +@asynccontextmanager +async def session_scope_for(db_service: _SupportsWithSession) -> AsyncGenerator[AsyncSession, None]: + """Async write session scope over ``db_service`` with auto-commit/rollback. + + Commits if the body completes without error; rolls back on exception. + ``HTTPException`` is treated as FastAPI control flow (rolled back but not + logged as an error); any other exception is logged then re-raised. + """ + async with db_service._with_session() as session: # noqa: SLF001 + try: + yield session + await session.commit() + except HTTPException: + # HTTPExceptions are control flow in FastAPI (returning 4xx/5xx responses), + # not actual errors. Don't log them - FastAPI's exception handlers will + # take care of the HTTP response. Just rollback any uncommitted changes. + if session.is_active: + from sqlalchemy.exc import InvalidRequestError + + with suppress(InvalidRequestError): + await session.rollback() + raise + except Exception as e: + # Actual application/database errors - log at error level + await logger.aexception("An error occurred during the session scope.", exception=e) + + # Only rollback if session is still in a valid state + if session.is_active: + from sqlalchemy.exc import InvalidRequestError + + with suppress(InvalidRequestError): + # Session was already rolled back by SQLAlchemy + await session.rollback() + raise + # No explicit close needed - _with_session() handles it + + +@asynccontextmanager +async def session_scope_readonly_for(db_service: _SupportsWithSession) -> AsyncGenerator[AsyncSession, None]: + """Async read-only session scope over ``db_service`` (no commit, no rollback).""" + async with db_service._with_session() as session: # noqa: SLF001 + yield session + # No commit - read-only + # No clean up - client is responsible (plus, read only sessions are not committed) + # No explicit close needed - _with_session() handles it diff --git a/src/lfx/src/lfx/services/deps.py b/src/lfx/src/lfx/services/deps.py index 7705732dccae..3f3d488610c0 100644 --- a/src/lfx/src/lfx/services/deps.py +++ b/src/lfx/src/lfx/services/deps.py @@ -3,11 +3,9 @@ from __future__ import annotations import threading -from contextlib import asynccontextmanager, suppress +from contextlib import asynccontextmanager from typing import TYPE_CHECKING, cast -from fastapi import HTTPException - from lfx.log.logger import logger from lfx.services.config_discovery import resolve_config_dir from lfx.services.schema import ServiceType @@ -241,34 +239,13 @@ async def session_scope() -> AsyncGenerator[AsyncSession, None]: Raises: Exception: If an error occurs during the session scope. """ - db_service = get_db_service() - async with db_service._with_session() as session: # noqa: SLF001 - try: - yield session - await session.commit() - except HTTPException: - # HTTPExceptions are control flow in FastAPI (returning 4xx/5xx responses), - # not actual errors. Don't log them - FastAPI's exception handlers will - # take care of the HTTP response. Just rollback any uncommitted changes. - if session.is_active: - from sqlalchemy.exc import InvalidRequestError - - with suppress(InvalidRequestError): - await session.rollback() - raise - except Exception as e: - # Actual application/database errors - log at error level - await logger.aexception("An error occurred during the session scope.", exception=e) - - # Only rollback if session is still in a valid state - if session.is_active: - from sqlalchemy.exc import InvalidRequestError - - with suppress(InvalidRequestError): - # Session was already rolled back by SQLAlchemy - await session.rollback() - raise - # No explicit close needed - _with_session() handles it + # Delegates to the shared, db-service-parameterized helper (Option B). The + # DatabaseService port also exposes this as ``db_service.session_scope()``; + # both routes call the same ``session_scope_for`` so semantics never drift. + from lfx.services.database.session import session_scope_for + + async with session_scope_for(get_db_service()) as session: + yield session async def injectable_session_scope_readonly(): @@ -286,9 +263,7 @@ async def session_scope_readonly() -> AsyncGenerator[AsyncSession, None]: Yields: AsyncSession: The async session object. """ - db_service = get_db_service() - async with db_service._with_session() as session: # noqa: SLF001 + from lfx.services.database.session import session_scope_readonly_for + + async with session_scope_readonly_for(get_db_service()) as session: yield session - # No commit - read-only - # No clean up - client is responsible (plus, read only sessions are not committed) - # No explicit close needed - _with_session() handles it diff --git a/src/lfx/src/lfx/services/interfaces.py b/src/lfx/src/lfx/services/interfaces.py index 5cbf8ebaf888..9b5988230399 100644 --- a/src/lfx/src/lfx/services/interfaces.py +++ b/src/lfx/src/lfx/services/interfaces.py @@ -79,13 +79,27 @@ async def api_key_security( class DatabaseServiceProtocol(Protocol): - """Protocol for database service.""" + """Protocol for database service (Tier 1 infrastructure port).""" @abstractmethod def with_session(self) -> Any: """Get database session.""" ... + @abstractmethod + def session_scope(self) -> Any: + """Async write session scope (auto-commit/rollback). + + Tier 2 services call this on their injected ``database_service`` to + persist/read the models they own. + """ + ... + + @abstractmethod + def session_scope_readonly(self) -> Any: + """Async read-only session scope (no commit/rollback).""" + ... + class StorageServiceProtocol(Protocol): """Protocol for storage service.""" diff --git a/src/lfx/src/lfx/services/manager.py b/src/lfx/src/lfx/services/manager.py index 86b16f303f24..c0408207b74f 100644 --- a/src/lfx/src/lfx/services/manager.py +++ b/src/lfx/src/lfx/services/manager.py @@ -10,13 +10,16 @@ from __future__ import annotations import asyncio +import hashlib import importlib import inspect +import json import threading from pathlib import Path from typing import TYPE_CHECKING from lfx.log.logger import logger +from lfx.services.capabilities import Capability, ServiceWiringError, Tier, WiringManifestEntry from lfx.services.config_discovery import ( get_nested_section, get_preferred_config_source, @@ -52,6 +55,9 @@ def __init__(self) -> None: self.keyed_lock = KeyedMemoryLockManager() self.factory_registered = False self._plugins_discovered = False + # Active resolution chain, used to detect dependency cycles during + # recursive service creation (A -> B -> A). + self._resolving_stack: list[ServiceType] = [] # Always register settings service from lfx.services.settings.factory import SettingsServiceFactory @@ -100,6 +106,13 @@ def register_service_class( logger.warning(msg) raise ValueError(msg) + # Reject classes that don't implement the declared port for this service + # type. Applies to every discovery source (decorator, config, entry + # point), so an unrelated class reusing a service key cannot silently + # replace a built-in service. + if not self._validate_port(service_type, service_class): + return + if service_type in self.service_classes and not override: logger.warning(f"Service {service_type.value} already registered. Use override=True to replace it.") return @@ -129,35 +142,50 @@ def _create_service(self, service_name: ServiceType, default: ServiceFactory | N """Create a new service given its name, handling dependencies.""" logger.debug(f"Create service {service_name}") - # Settings service is special - always use factory, never from plugins - if service_name == ServiceType.SETTINGS_SERVICE: - self._create_service_from_factory(service_name, default) - return + # Cycle guard: if we are already resolving this service further up the + # recursion, its dependency graph loops back on itself. + if service_name in self._resolving_stack: + chain = " -> ".join(s.value for s in [*self._resolving_stack, service_name]) + msg = f"Dependency cycle detected: {chain}" + raise ServiceWiringError(msg) - # Try plugin discovery first (if not already done) - if not self._plugins_discovered: - # Get config_dir from settings service if available - config_dir = None - if ServiceType.SETTINGS_SERVICE in self.services: - settings_service = self.services[ServiceType.SETTINGS_SERVICE] - if hasattr(settings_service, "settings") and settings_service.settings.config_dir: - config_dir = Path(settings_service.settings.config_dir) + self._resolving_stack.append(service_name) + try: + # Settings service is special - always use factory, never from plugins + if service_name == ServiceType.SETTINGS_SERVICE: + self._create_service_from_factory(service_name, default) + return - self.discover_plugins(config_dir) + # Try plugin discovery first (if not already done) + if not self._plugins_discovered: + # Get config_dir from settings service if available + config_dir = None + if ServiceType.SETTINGS_SERVICE in self.services: + settings_service = self.services[ServiceType.SETTINGS_SERVICE] + if hasattr(settings_service, "settings") and settings_service.settings.config_dir: + config_dir = Path(settings_service.settings.config_dir) - # Check if we have a direct service class registration (new system) - if service_name in self.service_classes: - self._create_service_from_class(service_name) - return + self.discover_plugins(config_dir) - # Fall back to factory-based creation (old system) - self._create_service_from_factory(service_name, default) + # Check if we have a direct service class registration (new system) + if service_name in self.service_classes: + self._create_service_from_class(service_name) + return + + # Fall back to factory-based creation (old system) + self._create_service_from_factory(service_name, default) + finally: + self._resolving_stack.pop() def _create_service_from_class(self, service_name: ServiceType) -> None: """Create a service from a registered service class (new plugin system).""" service_class = self.service_classes[service_name] logger.debug(f"Creating service from class: {service_name.value} -> {service_class.__name__}") + # Validate + pre-create declared `requires` dependencies (capabilities + + # tier layering enforced here, before instantiation). + self._resolve_requirements(service_name, service_class) + # Inspect __init__ to determine dependencies init_signature = inspect.signature(service_class.__init__) dependencies = {} @@ -205,6 +233,194 @@ def _create_service_from_class(self, service_name: ServiceType) -> None: logger.exception(f"Failed to create service {service_name.value}: {exc}") raise + # ------------------------------------------------------------------ + # Wiring: ports, capabilities, tiers, and boot-time validation + # ------------------------------------------------------------------ + + def _validate_port(self, service_type: ServiceType, service_class: type) -> bool: + """Return True if ``service_class`` implements the declared port for ``service_type``. + + Services without a declared port (not in ``SERVICE_PORTS``) are not + validated and always pass. A class that fails is logged and rejected + (not registered). + """ + from lfx.services.ports import get_expected_port + + expected = get_expected_port(service_type) + if expected is None: + return True + if isinstance(service_class, type) and issubclass(service_class, expected): + return True + logger.warning( + f"Service class {service_class!r} registered for {service_type.value} is not a subclass of " + f"{expected.__name__}; skipping registration to avoid replacing the built-in service with an " + f"incompatible implementation." + ) + return False + + def _resolve_class(self, service_type: ServiceType) -> type | None: + """Return the class that *would* be created for ``service_type``, without instantiating. + + Prefers a directly-registered service class (new system), then a + factory's ``service_class`` (old system). Returns ``None`` if nothing is + registered for the type. Used by wiring validation and the manifest so + the whole graph can be reasoned about before any service is built. + """ + if service_type in self.service_classes: + return self.service_classes[service_type] + factory = self.factories.get(service_type) + if factory is not None: + return getattr(factory, "service_class", None) + return None + + def _resolve_requirements(self, service_name: ServiceType, service_class: type) -> None: + """Validate declared ``requires`` and pre-create the dependencies. + + Raises ``ServiceWiringError`` if a dependency is unregistered, fails a + capability requirement, or violates the tier layering invariant + (Tier 1 must not depend on Tier 2). Dependencies that pass validation are + created in topological order so they exist when the dependent's + ``__init__`` runs. + """ + requires = getattr(service_class, "requires", ()) + if not requires: + return + + service_tier = getattr(service_class, "tier", None) + for req in requires: + dep_type = req.service + dep_class = self._resolve_class(dep_type) + if dep_class is None: + msg = ( + f"{service_name.value} requires {dep_type.value}, but no implementation is registered. " + f"Register a {dep_type.value} implementation (config, decorator, or entry point)." + ) + raise ServiceWiringError(msg) + + # Tier layering: a Tier 1 (infrastructure) service must not depend on + # a Tier 2 (composed) service. + dep_tier = getattr(dep_class, "tier", None) + if service_tier == Tier.INFRASTRUCTURE and dep_tier == Tier.COMPOSED: + msg = ( + f"Layering violation: Tier 1 service {service_name.value} declares a dependency on " + f"Tier 2 service {dep_type.value}. Infrastructure services may only depend on settings " + f"or other infrastructure services." + ) + raise ServiceWiringError(msg) + + # Capability enforcement: the resolved implementation must provide + # every capability the dependent requires of it. + dep_caps = self._capabilities_of(dep_class) + missing = set(req.capabilities) - dep_caps + if missing: + have = ", ".join(sorted(c.value for c in dep_caps)) or "none" + need = ", ".join(sorted(c.value for c in missing)) + msg = ( + f"{service_name.value} requires {dep_type.value} to provide capabilities [{need}], but the " + f"resolved implementation {dep_class.__name__} provides [{have}]." + ) + raise ServiceWiringError(msg) + + # Create the dependency (topological order) if not present yet. + if dep_type not in self.services: + self._create_service(dep_type) + + @staticmethod + def _capabilities_of(service_class: type | None) -> set[Capability]: + """Read a class's declared capabilities defensively (non-Service classes allowed).""" + return set(getattr(service_class, "capabilities", frozenset())) + + def wiring_manifest(self, *, discover: bool = True) -> dict[ServiceType, WiringManifestEntry]: + """Resolve every registered service type to its implementation, without instantiating. + + Returns a mapping of ``ServiceType`` -> ``WiringManifestEntry`` describing + the chosen implementation, its package, tier, and capabilities. Types with + no registered implementation are omitted. Useful for a ``/health/services`` + endpoint or to eyeball what is actually wired. + """ + if discover and not self._plugins_discovered: + self.discover_plugins() + + manifest: dict[ServiceType, WiringManifestEntry] = {} + for service_type in ServiceType: + impl = self._resolve_class(service_type) + if impl is None: + continue + tier = getattr(impl, "tier", None) + manifest[service_type] = WiringManifestEntry( + service_type=service_type.value, + impl_class=impl.__name__, + package=(impl.__module__ or "").split(".")[0], + tier=int(tier) if tier is not None else 0, + capabilities=frozenset(self._capabilities_of(impl)), + ) + return manifest + + def validate_wiring(self, *, discover: bool = True) -> dict[ServiceType, WiringManifestEntry]: + """Eagerly validate the whole service graph at boot; return the manifest. + + Walks every registered service's ``requires`` edges and enforces + dependency presence, capability requirements, and the tier layering + invariant — *without* instantiating any service. Raises + ``ServiceWiringError`` on the first violation so a misconfigured + deployment fails before accepting traffic rather than mid-request. + """ + if discover and not self._plugins_discovered: + self.discover_plugins() + + manifest = self.wiring_manifest(discover=False) + for service_type in list(self.service_classes) + list(self.factories): + # Normalize factory string keys / enum keys to ServiceType. + try: + st = service_type if isinstance(service_type, ServiceType) else ServiceType(service_type) + except ValueError: + continue + impl = self._resolve_class(st) + if impl is None: + continue + self._validate_requirements_static(st, impl) + return manifest + + def _validate_requirements_static(self, service_name: ServiceType, service_class: type) -> None: + """Validation-only variant of ``_resolve_requirements`` (creates nothing).""" + requires = getattr(service_class, "requires", ()) + service_tier = getattr(service_class, "tier", None) + for req in requires: + dep_class = self._resolve_class(req.service) + if dep_class is None: + msg = f"{service_name.value} requires {req.service.value}, but no implementation is registered." + raise ServiceWiringError(msg) + dep_tier = getattr(dep_class, "tier", None) + if service_tier == Tier.INFRASTRUCTURE and dep_tier == Tier.COMPOSED: + msg = ( + f"Layering violation: Tier 1 service {service_name.value} declares a dependency on " + f"Tier 2 service {req.service.value}." + ) + raise ServiceWiringError(msg) + missing = set(req.capabilities) - self._capabilities_of(dep_class) + if missing: + have = ", ".join(sorted(c.value for c in self._capabilities_of(dep_class))) or "none" + need = ", ".join(sorted(c.value for c in missing)) + msg = ( + f"{service_name.value} requires {req.service.value} to provide capabilities [{need}], but the " + f"resolved implementation {dep_class.__name__} provides [{have}]." + ) + raise ServiceWiringError(msg) + + def wiring_fingerprint(self, *, discover: bool = True) -> str: + """Stable hash of the wiring's *capabilities* (not implementation classes). + + Two deployments whose services advertise the same per-type capability + sets produce the same fingerprint even if the concrete classes differ + (e.g. langflow's memory class vs a cloud plugin's, both persistent). This + is what makes a builder-vs-production divergence check fire on behavioral + difference, not on the unavoidable fact that different packages own the + classes. ``impl_class`` / ``package`` are deliberately excluded. + """ + manifest = self.wiring_manifest(discover=discover) + payload = sorted((st.value, sorted(c.value for c in entry.capabilities)) for st, entry in manifest.items()) + return hashlib.sha256(json.dumps(payload, separators=(",", ":")).encode()).hexdigest() + def _resolve_service_type_from_annotation(self, annotation) -> ServiceType | None: """Resolve a ServiceType from a type annotation. @@ -409,35 +625,15 @@ def _discover_from_entry_points(self) -> None: ) return - # Map of service_type → expected base class. Used to reject malformed - # entry-point plugins before they take over an authorization-critical - # service. Without this check an arbitrary class from an unrelated - # PyPI package that happened to register the same entry-point name - # would silently replace the OSS implementation. - expected_bases: dict[ServiceType, type] = {} - try: - from lfx.services.authorization.base import BaseAuthorizationService - - expected_bases[ServiceType.AUTHORIZATION_SERVICE] = BaseAuthorizationService - except Exception as exc: # noqa: BLE001 — optional import, validation just skipped - logger.debug(f"BaseAuthorizationService unavailable; entry-point validation skipped: {exc}") - for ep in eps: try: service_class = ep.load() - # Entry point name should match ServiceType enum value + # Entry point name should match ServiceType enum value. service_type = ServiceType(ep.name) - expected_base = expected_bases.get(service_type) - if expected_base is not None and not ( - isinstance(service_class, type) and issubclass(service_class, expected_base) - ): - logger.warning( - f"Entry point {ep.name} resolved to {service_class!r}, " - f"which is not a subclass of {expected_base.__name__}. " - f"Skipping registration to avoid silently replacing the " - f"built-in service with an incompatible plugin." - ) - continue + # Port validation (must subclass the declared port) is enforced + # centrally in register_service_class for every discovery source, + # so an unrelated class reusing this entry-point name cannot + # silently replace a built-in service. self.register_service_class(service_type, service_class, override=False) logger.debug(f"Loaded service from entry point: {ep.name}") except (ValueError, AttributeError) as exc: diff --git a/src/lfx/src/lfx/services/memory/base.py b/src/lfx/src/lfx/services/memory/base.py index 2f87f71493e3..f87934fd84ee 100644 --- a/src/lfx/src/lfx/services/memory/base.py +++ b/src/lfx/src/lfx/services/memory/base.py @@ -14,9 +14,10 @@ from __future__ import annotations from abc import abstractmethod -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, ClassVar from lfx.services.base import Service +from lfx.services.capabilities import Tier from lfx.utils.async_helpers import run_until_complete if TYPE_CHECKING: @@ -43,6 +44,11 @@ class MemoryService(Service): name = "memory_service" + # Chat memory is a Tier 2 (composed) service: it owns behavior over the + # lfx-owned ``message`` model and delegates persistence to a Tier 1 service. + # Concrete backends declare their own ``capabilities`` and ``requires``. + tier: ClassVar[Tier] = Tier.COMPOSED + # --- Abstract primitives ------------------------------------------------- @abstractmethod diff --git a/src/lfx/src/lfx/services/memory/contract.py b/src/lfx/src/lfx/services/memory/contract.py new file mode 100644 index 000000000000..e75c84aa4c18 --- /dev/null +++ b/src/lfx/src/lfx/services/memory/contract.py @@ -0,0 +1,148 @@ +"""Importable behavioral contract for any ``MemoryService`` implementation. + +Every registered memory backend — the in-memory default, the database-backed +service, or a future plugin — must pass this same suite. This is the concrete +anti-divergence mechanism from the two-tier design: **semantics live in the +contract; implementations may differ only in quality attributes (capabilities), +never in observable behavior.** If the builder and a production wiring ever +behaved differently on chat memory, that gap is a missing contract test here, +not an ``if embedded:`` branch in the engine. + +Usage (in a test module):: + + class TestInMemoryContract(MemoryServiceContract): + async def build_memory_service(self): + return InMemoryMemoryService() + +The contract intentionally exercises the **facade** primitives that must be +identical across backends (``astore_message`` / ``aget_messages`` / +``aupdate_messages`` on existing rows / ``adelete_messages`` / +``adelete_message`` / ordering / session scoping). Lower-level behavior that is +legitimately implementation-specific (e.g. ``aupdate_messages`` against a +*missing* id — the in-memory store upserts, the database backend raises) is +deliberately left unspecified here; the facade ``astore_message`` reconciles it +and is what the contract pins. +""" +# This module is a shipped test-support mixin: asserts are the point of the +# contract, so S101 (assert usage) is allowed file-wide. +# ruff: noqa: S101 + +from __future__ import annotations + +import uuid + +import pytest + +from lfx.schema.message import Message +from lfx.services.memory.base import MemoryService + + +def _make_message(text: str, session_id: str, sender: str = "User", sender_name: str = "Alice") -> Message: + """Build a minimal valid Message (session_id/sender/sender_name are required).""" + return Message(text=text, session_id=session_id, sender=sender, sender_name=sender_name) + + +class MemoryServiceContract: + """Mixin of behavioral tests every MemoryService backend must satisfy. + + Subclasses implement ``build_memory_service`` to return a ready service. + """ + + async def build_memory_service(self) -> MemoryService: # pragma: no cover - overridden + """Return a ready-to-use memory service instance.""" + raise NotImplementedError + + @pytest.mark.asyncio + async def test_is_memory_service(self) -> None: + service = await self.build_memory_service() + assert isinstance(service, MemoryService) + + @pytest.mark.asyncio + async def test_store_and_get_roundtrip(self) -> None: + service = await self.build_memory_service() + session_id = f"s-{uuid.uuid4()}" + await service.astore_message(_make_message("hello", session_id)) + + got = await service.aget_messages(session_id=session_id) + assert len(got) == 1 + assert got[0].text == "hello" + + @pytest.mark.asyncio + async def test_store_requires_core_fields(self) -> None: + service = await self.build_memory_service() + bad = Message(text="no session", sender="User", sender_name="Alice") + with pytest.raises(ValueError, match="session_id"): + await service.astore_message(bad) + + @pytest.mark.asyncio + async def test_get_filters_by_sender(self) -> None: + service = await self.build_memory_service() + session_id = f"s-{uuid.uuid4()}" + await service.astore_message(_make_message("from user", session_id, sender="User", sender_name="Alice")) + await service.astore_message(_make_message("from machine", session_id, sender="Machine", sender_name="Bot")) + + only_user = await service.aget_messages(session_id=session_id, sender="User") + assert len(only_user) == 1 + assert only_user[0].text == "from user" + + @pytest.mark.asyncio + async def test_store_existing_id_updates_not_duplicates(self) -> None: + # Re-storing a message that already has an id updates the row in place + # rather than inserting a duplicate. We mutate ``sender_name`` (a plain + # field), not ``text`` — ``text`` is a computed field over + # ``content_blocks`` and does not round-trip through a serializing store, + # so asserting on it would test an operation the backends legitimately + # implement differently rather than a shared contract. + service = await self.build_memory_service() + session_id = f"s-{uuid.uuid4()}" + stored = await service.astore_message(_make_message("hi", session_id, sender_name="Alice")) + message = stored[0] + + message.sender_name = "Renamed" + await service.astore_message(message) + + got = await service.aget_messages(session_id=session_id) + assert len(got) == 1 + assert got[0].sender_name == "Renamed" + + @pytest.mark.asyncio + async def test_aupdate_existing_message(self) -> None: + service = await self.build_memory_service() + session_id = f"s-{uuid.uuid4()}" + stored = await service.astore_message(_make_message("original", session_id, sender_name="Alice")) + message = stored[0] + + message.sender_name = "Edited" + await service.aupdate_messages([message]) + + got = await service.aget_messages(session_id=session_id) + assert got[0].sender_name == "Edited" + + @pytest.mark.asyncio + async def test_delete_by_session_scopes_correctly(self) -> None: + service = await self.build_memory_service() + keep_session = f"keep-{uuid.uuid4()}" + drop_session = f"drop-{uuid.uuid4()}" + await service.astore_message(_make_message("keep me", keep_session)) + await service.astore_message(_make_message("drop me", drop_session)) + + await service.adelete_messages(session_id=drop_session) + + assert await service.aget_messages(session_id=drop_session) == [] + assert len(await service.aget_messages(session_id=keep_session)) == 1 + + @pytest.mark.asyncio + async def test_delete_single_message(self) -> None: + service = await self.build_memory_service() + session_id = f"s-{uuid.uuid4()}" + stored = await service.astore_message(_make_message("temp", session_id)) + + await service.adelete_message(str(stored[0].id)) + + assert await service.aget_messages(session_id=session_id) == [] + + @pytest.mark.asyncio + async def test_delete_requires_a_filter(self) -> None: + service = await self.build_memory_service() + with pytest.raises(ValueError, match="session_id or context_id"): + await service.adelete_messages() diff --git a/src/lfx/src/lfx/services/memory/database.py b/src/lfx/src/lfx/services/memory/database.py new file mode 100644 index 000000000000..4ba4ef24f41a --- /dev/null +++ b/src/lfx/src/lfx/services/memory/database.py @@ -0,0 +1,295 @@ +"""Database-backed chat-message memory — the reference Tier 2 service. + +This is the canonical example of the two-tier pluggable-service design: + +- It is **Tier 2 (composed)**: it owns the store/read/update/delete *behavior* + over the lfx-owned ``message`` model, but it does not talk to a database + directly. It **requires** the Tier 1 ``DATABASE_SERVICE`` and delegates every + commit/read to it. +- It uses **Option B (explicit injection)**: the database service is passed into + ``__init__`` (resolved and validated by the service manager from the + ``requires`` declaration) rather than fetched from a module-level global. The + service calls ``self.database_service.session_scope()`` — the promoted Tier 1 + port method. + +Persistence is inherited from whatever database service is wired underneath: +langflow's engine, ``lfx serve`` sqlite/Postgres, etc. The *same class* runs in +the builder and in the production worker-plane; only the Tier 1 service beneath +it changes. That is the convergence the two-tier model is designed to reach — +langflow selects this lfx implementation instead of authoring its own. + +The CRUD logic here is a faithful port of the historical ``langflow.memory`` +module (which already operated on the lfx ``MessageTable`` through a +langflow→lfx ``session_scope`` shim); the only change is that the session comes +from the injected Tier 1 service. +""" + +from __future__ import annotations + +import asyncio +import json +from typing import TYPE_CHECKING, ClassVar +from uuid import UUID + +from sqlalchemy import delete +from sqlmodel import col, select + +from lfx.log.logger import logger +from lfx.schema.message import Message +from lfx.services.capabilities import Capability, Requires +from lfx.services.database.models.message import MessageRead, MessageTable +from lfx.services.memory.base import MemoryService +from lfx.services.schema import ServiceType + +if TYPE_CHECKING: + from sqlmodel.ext.asyncio.session import AsyncSession + + from lfx.services.interfaces import DatabaseServiceProtocol + + +def _as_uuid(value): + """Coerce a value to UUID when possible (MessageTable.id is a UUID column). + + A bare str primary key fails SQLAlchemy's UUID bind processor, so ids that + arrive as strings (e.g. ``str(message.id)`` from a caller) are normalized + here. Non-UUID strings are returned unchanged so lookups simply miss rather + than raising. + """ + if isinstance(value, UUID): + return value + try: + return UUID(str(value)) + except (ValueError, AttributeError, TypeError): + return value + + +def _build_messages_query( + sender: str | None = None, + sender_name: str | None = None, + session_id: str | UUID | None = None, + context_id: str | None = None, + order_by: str | None = "timestamp", + order: str | None = "DESC", + flow_id: UUID | None = None, + limit: int | None = None, +): + """Build the filtered/ordered SELECT over ``MessageTable`` (error rows excluded).""" + stmt = select(MessageTable).where(MessageTable.error == False) # noqa: E712 + if sender: + stmt = stmt.where(MessageTable.sender == sender) + if sender_name: + stmt = stmt.where(MessageTable.sender_name == sender_name) + if session_id: + stmt = stmt.where(MessageTable.session_id == session_id) + if context_id: + stmt = stmt.where(MessageTable.context_id == context_id) + if flow_id: + stmt = stmt.where(MessageTable.flow_id == flow_id) + if order_by: + ordering = getattr(MessageTable, order_by).desc() if order == "DESC" else getattr(MessageTable, order_by).asc() + stmt = stmt.order_by(ordering) + if limit: + stmt = stmt.limit(limit) + return stmt + + +class DatabaseMemoryService(MemoryService): + """Chat-message memory backed by the injected Tier 1 database service.""" + + name = "memory_service" + + # Wired over a real (persistent) database, this backend persists and is + # queryable. SHARED is inherited from the underlying database in practice; + # the static declaration is the conservative {QUERYABLE, PERSISTENT}. + capabilities: ClassVar[frozenset[Capability]] = frozenset({Capability.QUERYABLE, Capability.PERSISTENT}) + + # Requires the database service to be *present* (decision: presence, not + # PERSISTENT — so this backend can also be wired over an ephemeral database + # without failing validate_wiring). The injected instance decides whether + # writes actually survive a restart. + requires: ClassVar[tuple[Requires, ...]] = (Requires(ServiceType.DATABASE_SERVICE),) + + def __init__(self, database_service: DatabaseServiceProtocol) -> None: + super().__init__() + # Option B: the Tier 1 dependency is injected by the manager, not fetched + # from a global. All persistence goes through this handle. + self.database_service = database_service + self.set_ready() + logger.debug("Database-backed memory service initialized") + + def _session_scope(self): + """Write session scope from the injected Tier 1 database service.""" + return self.database_service.session_scope() + + async def astore_message( + self, + message: Message, + flow_id: str | UUID | None = None, + run_id: str | UUID | None = None, + ) -> list[Message]: + """Store a single message. Updates in place when it already has an id.""" + if not message: + await logger.awarning("No message provided.") + return [] + + if not message.session_id or not message.sender or not message.sender_name: + msg = ( + f"All of session_id, sender, and sender_name must be provided. Session ID: {message.session_id}," + f" Sender: {message.sender}, Sender Name: {message.sender_name}" + ) + raise ValueError(msg) + + if getattr(message, "id", None): + # Has an id: update it if present, otherwise fall through to insert. + try: + return await self.aupdate_messages([message]) + except ValueError as e: + await logger.aerror(e) + if flow_id and not isinstance(flow_id, UUID): + flow_id = UUID(flow_id) + return await self.aadd_messages([message], flow_id=flow_id, run_id=run_id) + + async def aget_messages( + self, + sender: str | None = None, + sender_name: str | None = None, + session_id: str | UUID | None = None, + context_id: str | UUID | None = None, + order_by: str | None = "timestamp", + order: str | None = "DESC", + flow_id: UUID | None = None, + limit: int | None = None, + ) -> list[Message]: + """Retrieve messages matching the filters, newest-first by default.""" + async with self.database_service.session_scope_readonly() as session: + stmt = _build_messages_query(sender, sender_name, session_id, context_id, order_by, order, flow_id, limit) + messages = await session.exec(stmt) + return [await Message.create(**d.model_dump()) for d in messages] + + async def aupdate_messages(self, messages: Message | list[Message]) -> list[Message]: + """Update existing messages by id. Raises if a message id is not found.""" + if not isinstance(messages, list): + messages = [messages] + + async with self._session_scope() as session: + updated_messages: list[MessageTable] = [] + for message in messages: + msg = await session.get(MessageTable, _as_uuid(message.id)) + if msg: + msg = msg.sqlmodel_update(message.model_dump(exclude_unset=True, exclude_none=True)) + # Convert flow_id to UUID if it's a string, preventing a save error. + if msg.flow_id and isinstance(msg.flow_id, str): + msg.flow_id = UUID(msg.flow_id) + result = session.add(msg) + if asyncio.iscoroutine(result): + await result + updated_messages.append(msg) + else: + error_message = f"Message with id {message.id} not found" + await logger.awarning(error_message) + raise ValueError(error_message) + + return [MessageRead.model_validate(m, from_attributes=True) for m in updated_messages] + + async def aadd_messages( + self, + messages: Message | list[Message], + flow_id: str | UUID | None = None, + run_id: str | UUID | None = None, + ) -> list[Message]: + """Batch-insert messages in a single session.""" + if not isinstance(messages, list): + messages = [messages] + + for message in messages: + is_valid_message = isinstance(message, Message) or ( + hasattr(message, "__class__") and message.__class__.__name__ in ["Message", "ErrorMessage"] + ) + if not is_valid_message: + types = ", ".join([str(type(msg)) for msg in messages]) + msg = f"The messages must be instances of Message. Found: {types}" + raise ValueError(msg) + + try: + message_models = [MessageTable.from_message(m, flow_id=flow_id, run_id=run_id) for m in messages] + async with self._session_scope() as session: + message_models = await self._aadd_messagetables(message_models, session) + return [await Message.create(**m.model_dump()) for m in message_models] + except Exception as e: + await logger.aexception(e) + raise + + async def aadd_messagetables(self, messages: Message | list[Message]) -> list[Message]: + """Public batch entrypoint — same contract as ``aadd_messages``.""" + return await self.aadd_messages(messages) + + async def _aadd_messagetables( + self, + messages: list[MessageTable], + session: AsyncSession, + retry_count: int = 0, + ) -> list[MessageRead]: + """Add message rows with bounded retry on CancelledError, then normalize.""" + max_retries = 3 + try: + try: + for message in messages: + result = session.add(message) + if asyncio.iscoroutine(result): + await result + await session.commit() + except asyncio.CancelledError: + # build_public_tmp can raise CancelledError at commit where + # build_flow does not; retry a bounded number of times. + await session.rollback() + if retry_count >= max_retries: + await logger.awarning( + f"Max retries ({max_retries}) reached for _aadd_messagetables due to CancelledError" + ) + error_msg = "Add Message operation cancelled after multiple retries" + raise ValueError(error_msg) from None + return await self._aadd_messagetables(messages, session, retry_count + 1) + for message in messages: + await session.refresh(message) + except asyncio.CancelledError as e: + await logger.aexception(e) + error_msg = "Operation cancelled" + raise ValueError(error_msg) from e + except Exception as e: + await logger.aexception(e) + raise + + new_messages = [] + for msg in messages: + msg.properties = json.loads(msg.properties) if isinstance(msg.properties, str) else msg.properties # type: ignore[arg-type] + msg.content_blocks = [json.loads(j) if isinstance(j, str) else j for j in msg.content_blocks] # type: ignore[arg-type] + msg.category = msg.category or "" + new_messages.append(msg) + + return [MessageRead.model_validate(m, from_attributes=True) for m in new_messages] + + async def adelete_messages(self, session_id: str | None = None, context_id: str | None = None) -> None: + """Delete all messages for a session id or context id.""" + if not session_id and not context_id: + msg = "Either session_id or context_id must be provided to delete messages." + raise ValueError(msg) + + async with self._session_scope() as session: + filter_column = MessageTable.context_id if context_id else MessageTable.session_id + filter_value = context_id if context_id else session_id + stmt = ( + delete(MessageTable) + .where(col(filter_column) == filter_value) + .execution_options(synchronize_session="fetch") + ) + await session.exec(stmt) + + async def adelete_message(self, id_: str) -> None: + """Delete a single message by id (no-op if absent).""" + async with self._session_scope() as session: + message = await session.get(MessageTable, _as_uuid(id_)) + if message: + await session.delete(message) + + async def teardown(self) -> None: + """No owned resources — the Tier 1 database service owns the engine.""" diff --git a/src/lfx/src/lfx/services/memory/factory.py b/src/lfx/src/lfx/services/memory/factory.py index ddda35929ac2..4ed98855eae5 100644 --- a/src/lfx/src/lfx/services/memory/factory.py +++ b/src/lfx/src/lfx/services/memory/factory.py @@ -1,4 +1,12 @@ -"""Factory for the lean in-memory memory service used by bare lfx.""" +"""Factory for the lean in-memory memory service used by bare lfx. + +Registers ``InMemoryMemoryService`` as the zero-dependency default so a bare +``lfx run`` has working (ephemeral) chat memory. A host with a real database — +langflow, or ``lfx serve`` — selects the Tier 2 ``DatabaseMemoryService`` +instead by registering it as a service class (which takes precedence over this +factory), injecting its Tier 1 ``database_service``. See +``lfx.services.memory.database.DatabaseMemoryService``. +""" from __future__ import annotations @@ -9,14 +17,7 @@ class MemoryServiceFactory(ServiceFactory): - """Registers the no-deps ``InMemoryMemoryService`` as the lean default. - - The backend is chosen once, at creation time, from the registered database - service: with a real (non-noop) DB a DB-backed backend is appropriate, while - bare lfx without a database gets the round-tripping in-memory store. A heavier - backend (e.g. langflow's DB-backed memory service) overrides this through the - same service manager. - """ + """Registers the no-deps ``InMemoryMemoryService`` as the lean default.""" def __init__(self) -> None: super().__init__() @@ -25,13 +26,4 @@ def __init__(self) -> None: @override def create(self) -> InMemoryMemoryService: - from lfx.services.database.service import NoopDatabaseService - from lfx.services.deps import get_db_service - - if isinstance(get_db_service(), NoopDatabaseService): - return InMemoryMemoryService() - # TODO(follow-up): return DatabaseBackedMemoryService() for LFX executors - # (Postgres) once it lands. The seam is here so a non-noop DB — including a - # PG-configured bare-lfx run — resolves to the persistent backend without - # touching the ABC or the deps getter. return InMemoryMemoryService() diff --git a/src/lfx/src/lfx/services/memory/service.py b/src/lfx/src/lfx/services/memory/service.py index 9fcbdef90fdc..802fc8c47a37 100644 --- a/src/lfx/src/lfx/services/memory/service.py +++ b/src/lfx/src/lfx/services/memory/service.py @@ -8,10 +8,11 @@ from __future__ import annotations -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, ClassVar from uuid import UUID from lfx.log.logger import logger +from lfx.services.capabilities import Capability, Requires from lfx.services.memory.base import MemoryService if TYPE_CHECKING: @@ -19,10 +20,19 @@ class InMemoryMemoryService(MemoryService): - """Round-tripping in-memory memory backend (no database, no dependencies).""" + """Round-tripping in-memory memory backend (no database, no dependencies). + + The bare-lfx default. Store/read genuinely round-trips within the process, so + it is ``QUERYABLE`` but neither ``PERSISTENT`` nor ``SHARED`` — chat memory + works during an ``lfx run`` and is discarded when the process exits. It has no + Tier 1 dependency (``requires`` is empty). + """ name = "memory_service" + capabilities: ClassVar[frozenset[Capability]] = frozenset({Capability.QUERYABLE}) + requires: ClassVar[tuple[Requires, ...]] = () + def __init__(self) -> None: super().__init__() self._messages: dict[str, Message] = {} diff --git a/src/lfx/src/lfx/services/ports.py b/src/lfx/src/lfx/services/ports.py new file mode 100644 index 000000000000..77626e193690 --- /dev/null +++ b/src/lfx/src/lfx/services/ports.py @@ -0,0 +1,43 @@ +"""Canonical port (base class) for each pluggable ``ServiceType``. + +A *port* is the abstract base every implementation of a service type must +subclass. Registering a class that is not a subclass of the declared port is +rejected at registration time, for **all** discovery sources (decorator, config +file, entry point) — not just entry points. This prevents an unrelated class +(e.g. one from a third-party package that happens to reuse a service key) from +silently replacing a built-in service. + +Only service types with a formalized port are listed. Types absent from this map +are not validated (back-compat for services that predate the port model); the +map is intended to grow to cover every ``ServiceType`` as each port is written. + +Ports are stored as ``"module:ClassName"`` strings and imported lazily on first +use, so importing this module stays cheap and free of import cycles. +""" + +from __future__ import annotations + +from lfx.services.config_discovery import load_object_from_import_path +from lfx.services.schema import ServiceType + +# ServiceType -> "module:ClassName" of the abstract port each implementation +# must subclass. Grows one entry per port as services are formalized. +SERVICE_PORTS: dict[ServiceType, str] = { + ServiceType.MEMORY_SERVICE: "lfx.services.memory.base:MemoryService", + ServiceType.AUTHORIZATION_SERVICE: "lfx.services.authorization.base:BaseAuthorizationService", +} + + +def get_expected_port(service_type: ServiceType) -> type | None: + """Return the port base class for ``service_type``, or ``None`` if unlisted. + + Import failures are treated as "no port declared" (returns ``None``) so a + missing optional dependency degrades to no validation rather than blocking + registration entirely — the same lenient stance the manager already took for + the authorization port. + """ + import_path = SERVICE_PORTS.get(service_type) + if import_path is None: + return None + port = load_object_from_import_path(import_path, object_kind="port", object_key=service_type.value) + return port if isinstance(port, type) else None diff --git a/src/lfx/src/lfx/services/storage/postgres.py b/src/lfx/src/lfx/services/storage/postgres.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/src/lfx/tests/unit/services/memory/__init__.py b/src/lfx/tests/unit/services/memory/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/src/lfx/tests/unit/services/memory/test_memory_contract.py b/src/lfx/tests/unit/services/memory/test_memory_contract.py new file mode 100644 index 000000000000..8bd14d53c4bb --- /dev/null +++ b/src/lfx/tests/unit/services/memory/test_memory_contract.py @@ -0,0 +1,136 @@ +"""Run the shared MemoryService contract against every implementation. + +The whole point of the contract is that the *same* behavioral suite passes for +both the zero-dependency in-memory default and the database-backed Tier 2 +service. That equivalence is what lets langflow (DB-backed) and a bare +``lfx run`` (in-memory) share the same engine code — they differ in capabilities, +not behavior. + +``TestDatabaseMemoryContract`` also exercises the Option B wiring end to end: it +constructs a real (sqlite) Tier 1 database service and lets the service manager +inject it into ``DatabaseMemoryService``. +""" + +from __future__ import annotations + +from contextlib import asynccontextmanager + +import pytest +from lfx.services.capabilities import Capability, Tier +from lfx.services.database.models.message import MessageTable +from lfx.services.database.session import session_scope_for, session_scope_readonly_for +from lfx.services.manager import ServiceManager +from lfx.services.memory.contract import MemoryServiceContract +from lfx.services.memory.database import DatabaseMemoryService +from lfx.services.memory.service import InMemoryMemoryService +from lfx.services.schema import ServiceType +from sqlalchemy.ext.asyncio import create_async_engine +from sqlalchemy.pool import StaticPool +from sqlmodel import SQLModel +from sqlmodel.ext.asyncio.session import AsyncSession as SQLModelAsyncSession + + +class _SqliteDatabaseService: + """Minimal real Tier 1 database service over an in-memory sqlite engine. + + Implements exactly the port surface DatabaseMemoryService uses: ``_with_session`` + plus the promoted ``session_scope`` / ``session_scope_readonly`` methods. Declares + ``PERSISTENT`` — within the test process the sqlite db genuinely round-trips. + """ + + name = "database_service" + tier = Tier.INFRASTRUCTURE + capabilities = frozenset({Capability.PERSISTENT}) + + def __init__(self, engine) -> None: + self._engine = engine + self._ready = False + + def set_ready(self) -> None: # created via the factory path in the manager + self._ready = True + + async def teardown(self) -> None: + await self._engine.dispose() + + @asynccontextmanager + async def _with_session(self): + async with SQLModelAsyncSession(self._engine, expire_on_commit=False) as session: + yield session + + def session_scope(self): + return session_scope_for(self) + + def session_scope_readonly(self): + return session_scope_readonly_for(self) + + +async def _make_sqlite_db_service() -> _SqliteDatabaseService: + """Create a StaticPool in-memory sqlite engine with the message table.""" + engine = create_async_engine( + "sqlite+aiosqlite://", + poolclass=StaticPool, + connect_args={"check_same_thread": False}, + ) + async with engine.begin() as conn: + # Only the message table is needed (it has no cross-table FKs). + await conn.run_sync(lambda c: SQLModel.metadata.create_all(c, tables=[MessageTable.__table__])) + return _SqliteDatabaseService(engine) + + +class TestInMemoryMemoryContract(MemoryServiceContract): + """The bare-lfx in-memory default satisfies the contract.""" + + async def build_memory_service(self): + return InMemoryMemoryService() + + +class TestDatabaseMemoryContract(MemoryServiceContract): + """The DB-backed Tier 2 service satisfies the same contract over real sqlite.""" + + async def build_memory_service(self): + db_service = await _make_sqlite_db_service() + return DatabaseMemoryService(database_service=db_service) + + +async def test_manager_injects_database_service_option_b(): + """The manager resolves and injects the Tier 1 db dependency (Option B).""" + db_service = await _make_sqlite_db_service() + + class _FixedDBFactory: + service_class = _SqliteDatabaseService + + def __init__(self): + self.dependencies = [] + + def create(self): + return db_service + + mgr = ServiceManager() + mgr.factories[ServiceType.DATABASE_SERVICE.value] = _FixedDBFactory() + mgr.register_service_class(ServiceType.MEMORY_SERVICE, DatabaseMemoryService, override=True) + + memory = mgr.get(ServiceType.MEMORY_SERVICE) + assert isinstance(memory, DatabaseMemoryService) + # The injected dependency is the exact Tier 1 instance the manager resolved. + assert memory.database_service is db_service + + +async def test_bare_lfx_memory_requires_no_database(): + """InMemoryMemoryService declares no requirements and needs no DB to wire.""" + mgr = ServiceManager() + mgr.register_service_class(ServiceType.MEMORY_SERVICE, InMemoryMemoryService, override=True) + memory = mgr.get(ServiceType.MEMORY_SERVICE) + assert isinstance(memory, InMemoryMemoryService) + + +@pytest.mark.parametrize( + ("impl", "expected_caps"), + [ + (InMemoryMemoryService, {Capability.QUERYABLE}), + (DatabaseMemoryService, {Capability.QUERYABLE, Capability.PERSISTENT}), + ], +) +def test_memory_impls_declare_expected_capabilities(impl, expected_caps): + """Both memory backends are Tier 2 and advertise the intended capabilities.""" + assert impl.tier == Tier.COMPOSED + assert set(impl.capabilities) == expected_caps diff --git a/src/lfx/tests/unit/services/test_service_wiring.py b/src/lfx/tests/unit/services/test_service_wiring.py new file mode 100644 index 000000000000..794ac8e89ebb --- /dev/null +++ b/src/lfx/tests/unit/services/test_service_wiring.py @@ -0,0 +1,239 @@ +"""Tests for the two-tier service wiring machinery on the ServiceManager. + +Covers the mechanisms that make the pluggable service seam safe: +port validation (all sources), declared ``requires`` with capability +enforcement, the tier layering invariant, dependency-cycle detection, and the +capability-based wiring manifest / fingerprint. +""" + +from __future__ import annotations + +import pytest +from lfx.services.base import Service +from lfx.services.capabilities import Capability, Requires, ServiceWiringError, Tier +from lfx.services.manager import ServiceManager +from lfx.services.schema import ServiceType + + +class _Svc(Service): + """Minimal concrete Service for building synthetic graphs.""" + + _name = "cache_service" + + @property + def name(self) -> str: + return self._name + + async def teardown(self) -> None: + return None + + +def _service(name, *, tier=None, capabilities=frozenset(), requires=(), init=None): + """Build a Service subclass with the given wiring declarations.""" + ns = { + "name": name, + "tier": tier, + "capabilities": capabilities, + "requires": requires, + "teardown": _Svc.teardown, + } + if init is not None: + ns["__init__"] = init + return type(f"Svc_{name}", (Service,), ns) + + +# -------------------------------------------------------------------------- +# Port validation +# -------------------------------------------------------------------------- + + +def test_port_validation_rejects_non_subclass(): + """A class that is not a MemoryService subclass is refused for MEMORY_SERVICE.""" + mgr = ServiceManager() + + class NotAMemoryService: + name = "memory_service" + + mgr.register_service_class(ServiceType.MEMORY_SERVICE, NotAMemoryService, override=True) + assert ServiceType.MEMORY_SERVICE not in mgr.service_classes + + +def test_port_validation_accepts_real_subclass(): + """The real InMemoryMemoryService passes port validation.""" + from lfx.services.memory.service import InMemoryMemoryService + + mgr = ServiceManager() + mgr.register_service_class(ServiceType.MEMORY_SERVICE, InMemoryMemoryService, override=True) + assert mgr.service_classes[ServiceType.MEMORY_SERVICE] is InMemoryMemoryService + + +def test_unlisted_service_type_skips_port_validation(): + """A service type with no declared port registers without validation.""" + mgr = ServiceManager() + + class AnyCache(Service): + name = "cache_service" + + async def teardown(self) -> None: + return None + + mgr.register_service_class(ServiceType.CACHE_SERVICE, AnyCache, override=True) + assert mgr.service_classes[ServiceType.CACHE_SERVICE] is AnyCache + + +# -------------------------------------------------------------------------- +# Capability enforcement +# -------------------------------------------------------------------------- + + +def test_missing_capability_raises_wiring_error(): + """A dependent requiring PERSISTENT fails against a capability-less dependency.""" + weak_db = _service("database_service", tier=Tier.INFRASTRUCTURE, capabilities=frozenset()) + mem = _service( + "memory_service", + tier=Tier.COMPOSED, + requires=(Requires(ServiceType.DATABASE_SERVICE, frozenset({Capability.PERSISTENT})),), + ) + mgr = ServiceManager() + mgr.service_classes[ServiceType.DATABASE_SERVICE] = weak_db + mgr.service_classes[ServiceType.MEMORY_SERVICE] = mem + + with pytest.raises(ServiceWiringError, match="persistent"): + mgr.validate_wiring(discover=False) + + +def test_satisfied_capability_passes(): + """The same requirement passes against a PERSISTENT-capable dependency.""" + strong_db = _service("database_service", tier=Tier.INFRASTRUCTURE, capabilities=frozenset({Capability.PERSISTENT})) + mem = _service( + "memory_service", + tier=Tier.COMPOSED, + requires=(Requires(ServiceType.DATABASE_SERVICE, frozenset({Capability.PERSISTENT})),), + ) + mgr = ServiceManager() + mgr.service_classes[ServiceType.DATABASE_SERVICE] = strong_db + mgr.service_classes[ServiceType.MEMORY_SERVICE] = mem + + manifest = mgr.validate_wiring(discover=False) + assert manifest[ServiceType.MEMORY_SERVICE].impl_class == mem.__name__ + + +def test_presence_only_requirement_passes_against_capability_less_dep(): + """A presence-only Requires (empty capability set) accepts any implementation.""" + noop_db = _service("database_service", tier=Tier.INFRASTRUCTURE, capabilities=frozenset()) + mem = _service("memory_service", tier=Tier.COMPOSED, requires=(Requires(ServiceType.DATABASE_SERVICE),)) + mgr = ServiceManager() + mgr.service_classes[ServiceType.DATABASE_SERVICE] = noop_db + mgr.service_classes[ServiceType.MEMORY_SERVICE] = mem + + # No raise: memory requires the DB to be present, not persistent. + mgr.validate_wiring(discover=False) + + +def test_missing_dependency_raises(): + """A required dependency with no registered implementation fails validation.""" + mem = _service("memory_service", tier=Tier.COMPOSED, requires=(Requires(ServiceType.DATABASE_SERVICE),)) + mgr = ServiceManager() + mgr.service_classes[ServiceType.MEMORY_SERVICE] = mem + + with pytest.raises(ServiceWiringError, match="no implementation is registered"): + mgr.validate_wiring(discover=False) + + +# -------------------------------------------------------------------------- +# Tier layering invariant +# -------------------------------------------------------------------------- + + +def test_tier1_requiring_tier2_is_a_layering_violation(): + """A Tier 1 service that depends on a Tier 2 service fails validation.""" + bad_infra = _service("database_service", tier=Tier.INFRASTRUCTURE, requires=(Requires(ServiceType.MEMORY_SERVICE),)) + some_mem = _service("memory_service", tier=Tier.COMPOSED) + mgr = ServiceManager() + mgr.service_classes[ServiceType.DATABASE_SERVICE] = bad_infra + mgr.service_classes[ServiceType.MEMORY_SERVICE] = some_mem + + with pytest.raises(ServiceWiringError, match="Layering violation"): + mgr.validate_wiring(discover=False) + + +def test_tier2_may_depend_on_tier1(): + """A Tier 2 service depending on a Tier 1 service is allowed.""" + db = _service("database_service", tier=Tier.INFRASTRUCTURE) + mem = _service("memory_service", tier=Tier.COMPOSED, requires=(Requires(ServiceType.DATABASE_SERVICE),)) + mgr = ServiceManager() + mgr.service_classes[ServiceType.DATABASE_SERVICE] = db + mgr.service_classes[ServiceType.MEMORY_SERVICE] = mem + mgr.validate_wiring(discover=False) + + +# -------------------------------------------------------------------------- +# Cycle detection +# -------------------------------------------------------------------------- + + +def test_dependency_cycle_detected_on_create(): + """A -> B -> A cycle raises ServiceWiringError during creation.""" + + def init_a(self, storage_service): + self.storage_service = storage_service + + def init_b(self, cache_service): + self.cache_service = cache_service + + svc_a = _service("cache_service", init=init_a) + svc_b = _service("storage_service", init=init_b) + mgr = ServiceManager() + mgr.service_classes[ServiceType.CACHE_SERVICE] = svc_a + mgr.service_classes[ServiceType.STORAGE_SERVICE] = svc_b + + with pytest.raises(ServiceWiringError, match="cycle"): + mgr.get(ServiceType.CACHE_SERVICE) + + +# -------------------------------------------------------------------------- +# Manifest and fingerprint +# -------------------------------------------------------------------------- + + +def test_fingerprint_is_capability_based_not_class_based(): + """Two different classes with identical capabilities share a fingerprint.""" + caps = frozenset({Capability.QUERYABLE, Capability.PERSISTENT}) + mem_a = _service("memory_service", tier=Tier.COMPOSED, capabilities=caps) + mem_b = _service("memory_service", tier=Tier.COMPOSED, capabilities=caps) + + def fp(cls): + mgr = ServiceManager() + mgr.service_classes[ServiceType.MEMORY_SERVICE] = cls + return mgr.wiring_fingerprint(discover=False) + + assert mem_a is not mem_b + assert fp(mem_a) == fp(mem_b) + + +def test_fingerprint_changes_when_capabilities_differ(): + """Adding a capability changes the fingerprint (behavioral divergence is caught).""" + base = frozenset({Capability.QUERYABLE, Capability.PERSISTENT}) + mem_base = _service("memory_service", tier=Tier.COMPOSED, capabilities=base) + mem_shared = _service("memory_service", tier=Tier.COMPOSED, capabilities=base | {Capability.SHARED}) + + def fp(cls): + mgr = ServiceManager() + mgr.service_classes[ServiceType.MEMORY_SERVICE] = cls + return mgr.wiring_fingerprint(discover=False) + + assert fp(mem_base) != fp(mem_shared) + + +def test_manifest_reports_impl_tier_and_capabilities(): + """The manifest records the resolved class, tier, and capabilities per type.""" + from lfx.services.memory.service import InMemoryMemoryService + + mgr = ServiceManager() + mgr.service_classes[ServiceType.MEMORY_SERVICE] = InMemoryMemoryService + manifest = mgr.wiring_manifest(discover=False) + + entry = manifest[ServiceType.MEMORY_SERVICE] + assert entry.impl_class == "InMemoryMemoryService" + assert entry.tier == int(Tier.COMPOSED) + assert Capability.QUERYABLE in entry.capabilities diff --git a/uv.lock b/uv.lock index 896e2028da21..86229c765167 100644 --- a/uv.lock +++ b/uv.lock @@ -9289,9 +9289,11 @@ toolguard = [ [package.dev-dependencies] dev = [ + { name = "aiosqlite" }, { name = "asgi-lifespan" }, { name = "blockbuster" }, { name = "coverage" }, + { name = "greenlet" }, { name = "hypothesis" }, { name = "pytest" }, { name = "pytest-asyncio" }, @@ -9367,9 +9369,11 @@ provides-extras = ["opendsstar", "cassandra", "toolguard", "bundles"] [package.metadata.requires-dev] dev = [ + { name = "aiosqlite", specifier = ">=0.20.0" }, { name = "asgi-lifespan", specifier = ">=2.1.0" }, { name = "blockbuster", specifier = ">=1.5.25" }, { name = "coverage", specifier = ">=7.9.2" }, + { name = "greenlet", specifier = ">=3.0.0" }, { name = "hypothesis", specifier = ">=6.136.3" }, { name = "pytest", specifier = ">=9.0.3" }, { name = "pytest-asyncio", specifier = ">=0.26.0" }, From 22377515d17d2375f393e534aad244578b3543bd Mon Sep 17 00:00:00 2001 From: Debojit Kaushik Date: Mon, 13 Jul 2026 18:54:48 -0400 Subject: [PATCH 4/4] Split migration stream ownerships between LFX and LF. Moved service definitions to LFX from langflow-base. --- AGENTS.md | 2 +- pyproject.toml | 1 + .../helpers/windows_postgres_helper.py | 63 +- .../langflow/services/database/service.py | 699 ++------------- ...t_database_windows_postgres_integration.py | 23 +- src/lfx/PLUGGABLE_SERVICES.md | 55 ++ src/lfx/deploy/README.md | 65 ++ src/lfx/deploy/k8s/configmap.yaml | 42 + src/lfx/deploy/k8s/knative-service.yaml | 63 ++ src/lfx/deploy/k8s/secret.example.yaml | 25 + src/lfx/deploy/lfx.toml | 22 + src/lfx/pyproject.toml | 10 + src/lfx/src/lfx/__main__.py | 2 + src/lfx/src/lfx/cli/_db_commands.py | 121 +++ src/lfx/src/lfx/cli/commands.py | 67 ++ .../src/lfx/services/database/constants.py | 19 + .../services/database/migrations/README.md | 42 + .../services/database/migrations/alembic.ini | 49 + .../lfx/services/database/migrations/env.py | 209 +++++ .../database/migrations/script.py.mako | 30 + ...3e_initial_lfx_execution_history_schema.py | 91 ++ src/lfx/src/lfx/services/database/service.py | 839 +++++++++++++++++- .../src/lfx/utils/windows_postgres_helper.py | 55 ++ .../tests/unit/services/database/__init__.py | 0 .../database/test_database_service.py | 144 +++ uv.lock | 4 + 26 files changed, 2051 insertions(+), 691 deletions(-) create mode 100644 src/lfx/deploy/README.md create mode 100644 src/lfx/deploy/k8s/configmap.yaml create mode 100644 src/lfx/deploy/k8s/knative-service.yaml create mode 100644 src/lfx/deploy/k8s/secret.example.yaml create mode 100644 src/lfx/deploy/lfx.toml create mode 100644 src/lfx/src/lfx/cli/_db_commands.py create mode 100644 src/lfx/src/lfx/services/database/constants.py create mode 100644 src/lfx/src/lfx/services/database/migrations/README.md create mode 100644 src/lfx/src/lfx/services/database/migrations/alembic.ini create mode 100644 src/lfx/src/lfx/services/database/migrations/env.py create mode 100644 src/lfx/src/lfx/services/database/migrations/script.py.mako create mode 100644 src/lfx/src/lfx/services/database/migrations/versions/35f161a1123e_initial_lfx_execution_history_schema.py create mode 100644 src/lfx/src/lfx/utils/windows_postgres_helper.py create mode 100644 src/lfx/tests/unit/services/database/__init__.py create mode 100644 src/lfx/tests/unit/services/database/test_database_service.py diff --git a/AGENTS.md b/AGENTS.md index 149810277a94..3ed3e0f0c5bc 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -89,7 +89,7 @@ src/ ### ORM Model Ownership -All `table=True` SQLModel classes live in `lfx.services.database.models` (flat modules per domain: `flow`, `user`, `folder`, `api_key`, `variable`, `file`, `jobs`, `traces`, `message`, `transactions`, `vertex_builds`, etc.; `auth/` and `deployment_provider_account/` are subpackages). The old `langflow.services.database.models.*` paths remain as re-export shims (class identity preserved — both import paths yield the same class objects, so `SQLModel.metadata` registers each table once). CRUD modules, alembic migrations, and the DB service stay in `langflow-base`; `lfx` owns only the model definitions. Shared model helpers live in `lfx.services.database.utils` (string validators) and `lfx.services.database.models.variable` (the variable type constants); the langflow twins re-export them. `lfx` depends on `sqlmodel` for these models; import them lazily on lfx hot paths. Known seam wart: `lfx.services.database.models.memory_base` lazily imports `langflow.services.memory_base.preprocessing` inside one default-factory path (embedded-only). +All `table=True` SQLModel classes live in `lfx.services.database.models` (flat modules per domain: `flow`, `user`, `folder`, `api_key`, `variable`, `file`, `jobs`, `traces`, `message`, `transactions`, `vertex_builds`, etc.; `auth/` and `deployment_provider_account/` are subpackages). The old `langflow.services.database.models.*` paths remain as re-export shims (class identity preserved — both import paths yield the same class objects, so `SQLModel.metadata` registers each table once). The Tier 1 **DatabaseService** (engine/session/migration mechanism) now lives in `lfx.services.database.service`; `langflow.services.database.service.DatabaseService` is a thin subclass that overrides only its migration-stream identity (version table `alembic_version`, `script_location` → `langflow/alembic`) and adds domain bootstrap (superuser assignment, schema-health). `lfx` also owns its **own** alembic lineage (`lfx.services.database.migrations`, version table `lfx_alembic_version`, execution tables only) so bare `lfx serve` can provision its schema without langflow; drive it with `lfx db upgrade/check/...`. langflow keeps its 82-migration alembic lineage and the CRUD modules. The model classes are the single source of truth, so the two streams can only differ in staleness — caught by `alembic check` per stream. See `src/lfx/PLUGGABLE_SERVICES.md` (two-stream migration model) and `src/lfx/deploy/` (hydration). `lfx` owns the model definitions. Shared model helpers live in `lfx.services.database.utils` (string validators) and `lfx.services.database.models.variable` (the variable type constants); the langflow twins re-export them. `lfx` depends on `sqlmodel` for these models; import them lazily on lfx hot paths. Known seam wart: `lfx.services.database.models.memory_base` lazily imports `langflow.services.memory_base.preprocessing` inside one default-factory path (embedded-only). ### Service Layer Backend services in `src/backend/base/langflow/services/`: diff --git a/pyproject.toml b/pyproject.toml index 4667d2ad922b..fe0cffeee32d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -301,6 +301,7 @@ directory = "coverage" target-version = "py310" exclude = [ "src/backend/base/langflow/alembic/*", + "src/lfx/src/lfx/services/database/migrations/*", "src/frontend/tests/assets/*", "src/lfx/src/lfx/_assets/component_index.json", "docs/**/API-Reference/python-examples/**", diff --git a/src/backend/base/langflow/helpers/windows_postgres_helper.py b/src/backend/base/langflow/helpers/windows_postgres_helper.py index 1392fe4b483d..b5c6333e98ac 100644 --- a/src/backend/base/langflow/helpers/windows_postgres_helper.py +++ b/src/backend/base/langflow/helpers/windows_postgres_helper.py @@ -1,45 +1,18 @@ -"""Helper for Windows + PostgreSQL event loop configuration.""" - -import asyncio -import os -import platform - -from lfx.log.logger import logger - -LANGFLOW_DATABASE_URL = "LANGFLOW_DATABASE_URL" -POSTGRESQL_PREFIXES = ("postgresql", "postgres") - - -def configure_windows_postgres_event_loop(source: str | None = None) -> bool: - """Configure event loop for Windows + PostgreSQL compatibility. - - Args: - source: Optional identifier for logging context - - Returns: - True if configuration was applied, False otherwise - """ - if platform.system() != "Windows": - return False - - db_url = os.environ.get(LANGFLOW_DATABASE_URL, "") - if not db_url or not any(db_url.startswith(prefix) for prefix in POSTGRESQL_PREFIXES): - return False - - # Use getattr to safely access the Windows-only class on all platforms - selector_policy = getattr(asyncio, "WindowsSelectorEventLoopPolicy", None) - if selector_policy is None: - return False - - current_policy = asyncio.get_event_loop_policy() - if isinstance(current_policy, selector_policy): - return False - - asyncio.set_event_loop_policy(selector_policy()) - - log_context = {"event_loop": "WindowsSelectorEventLoop", "reason": "psycopg_compatibility"} - if source: - log_context["source"] = source - - logger.debug("Windows PostgreSQL event loop configured", extra=log_context) - return True +"""Helper for Windows + PostgreSQL event loop configuration. + +The implementation moved to ``lfx.utils.windows_postgres_helper`` when the Tier 1 +DatabaseService was extracted into lfx. This shim preserves the historical +``langflow.helpers.windows_postgres_helper`` import path. +""" + +from lfx.utils.windows_postgres_helper import ( + LANGFLOW_DATABASE_URL, + POSTGRESQL_PREFIXES, + configure_windows_postgres_event_loop, +) + +__all__ = [ + "LANGFLOW_DATABASE_URL", + "POSTGRESQL_PREFIXES", + "configure_windows_postgres_event_loop", +] diff --git a/src/backend/base/langflow/services/database/service.py b/src/backend/base/langflow/services/database/service.py index edd8b651dea1..df3819d99cd4 100644 --- a/src/backend/base/langflow/services/database/service.py +++ b/src/backend/base/langflow/services/database/service.py @@ -1,503 +1,83 @@ +"""langflow's DatabaseService: a thin subclass over lfx's Tier 1 service. + +The engine/session/migration *mechanism* now lives in +``lfx.services.database.service.DatabaseService`` (the Tier 1 infrastructure +port). langflow subclasses it to: + +* drive its **own** alembic lineage (``script_location`` -> ``langflow/alembic``, + version table ``alembic_version``), which owns the full schema; and +* add its domain bootstrap (superuser assignment, schema-health checks) that + references langflow-domain models. + +This is an intentional "override" -- the exception the convergence rule allows -- +because langflow's migration lineage and domain policy genuinely differ from a +bare ``lfx serve``. Everything else is inherited. The module also re-exports the +symbols callers historically imported from here so existing imports keep working. +See ``src/lfx/PLUGGABLE_SERVICES.md`` (two-stream migration model). +""" + from __future__ import annotations -import asyncio -import os import re -import sqlite3 -import sys -import time -from contextlib import asynccontextmanager, contextmanager, nullcontext -from datetime import datetime, timezone from pathlib import Path -from typing import TYPE_CHECKING, ClassVar +from typing import ClassVar -import anyio import sqlalchemy as sa -from alembic import command, util -from alembic.config import Config from lfx.log.logger import logger -from lfx.services.capabilities import Capability, Tier +from lfx.services.database.service import ( + DatabaseService as LfxDatabaseService, +) +from lfx.services.database.service import ( + UnsupportedPostgreSQLVersionError, + check_postgresql_version_sync, + check_sqlite_database_path, + get_sqlite_database_file_path, +) from lfx.services.deps import session_scope -from sqlalchemy import event, inspect -from sqlalchemy.dialects import sqlite as dialect_sqlite -from sqlalchemy.engine import Engine, make_url -from sqlalchemy.exc import OperationalError -from sqlalchemy.ext.asyncio import AsyncEngine, async_sessionmaker, create_async_engine -from sqlmodel import SQLModel, select, text -from sqlmodel.ext.asyncio.session import AsyncSession as SQLModelAsyncSession -from tenacity import retry, stop_after_attempt, wait_fixed - -from langflow.helpers.windows_postgres_helper import configure_windows_postgres_event_loop +from sqlalchemy import inspect +from sqlmodel import SQLModel, select + from langflow.initial_setup.constants import STARTER_FOLDER_NAME -from langflow.services.base import Service from langflow.services.database import models -from langflow.services.database.constants import ( - MIN_POSTGRESQL_MAJOR_VERSION, - POSTGRESQL_VERSION_REQUIRED_MESSAGE, -) from langflow.services.database.models.user.crud import get_user_by_username -from langflow.services.database.session import NoopSession from langflow.services.database.utils import Result, TableResults from langflow.services.deps import get_settings_service from langflow.services.utils import teardown_superuser -if TYPE_CHECKING: - from lfx.services.settings.service import SettingsService - - -class UnsupportedPostgreSQLVersionError(Exception): - """Raised when the PostgreSQL version is below the minimum required.""" - - -_PG_VERSION_QUERY = sa.text("SELECT current_setting('server_version_num'), current_setting('server_version')") - -# Stable namespace for the schema-migration advisory lock. The lock serializes -# concurrent ``alembic upgrade`` runs across workers so they do not race to -# CREATE TYPE / CREATE TABLE on a fresh database. Picked once and never changed -# so independent processes converge on the same lock; the value itself is -# arbitrary, just has to fit in a Postgres bigint and not collide with other -# advisory locks the application takes (currently none). -_MIGRATION_ADVISORY_LOCK_ID = 0x4C616E67666C6F77 # ASCII "Langflow" -_MIGRATION_LOCK_DEFAULT_TIMEOUT_S = 300.0 -_MIGRATION_LOCK_POLL_INTERVAL_S = 2.0 - - -def _migration_lock_timeout_s() -> float: - raw = os.getenv("LANGFLOW_MIGRATION_LOCK_TIMEOUT_S") - if raw is None: - return _MIGRATION_LOCK_DEFAULT_TIMEOUT_S - try: - return float(raw) - except ValueError: - logger.warning( - "Ignoring invalid LANGFLOW_MIGRATION_LOCK_TIMEOUT_S=%r; falling back to %.0fs.", - raw, - _MIGRATION_LOCK_DEFAULT_TIMEOUT_S, - ) - return _MIGRATION_LOCK_DEFAULT_TIMEOUT_S - - -def _acquire_migration_lock_or_raise(conn, lock_id: int) -> None: - """Acquire the advisory lock with a bounded wait, logging progress. - - Blocking ``pg_advisory_lock`` has no upper bound and ``lock_timeout`` does - not apply to advisory locks, so a worker hung mid-migration would silently - block every other worker forever. Instead poll ``pg_try_advisory_lock`` with - a configurable timeout and log when we're waiting, so operators see why - boot is stuck. - """ - if conn.execute(sa.text(f"SELECT pg_try_advisory_lock({lock_id})")).scalar(): - return - - timeout = _migration_lock_timeout_s() - logger.info( - "Migration advisory lock %s held by another worker; waiting up to %.0fs.", - lock_id, - timeout, +# Re-exported for backward compatibility (historical import site). +__all__ = [ + "DatabaseService", + "SQLModel", + "UnsupportedPostgreSQLVersionError", + "check_postgresql_version_sync", + "check_sqlite_database_path", + "get_sqlite_database_file_path", +] + + +class DatabaseService(LfxDatabaseService): + """langflow's Tier 1 database service: lfx engine/migration core + domain bootstrap.""" + + # langflow owns the full schema through its historical lineage; keep the + # default alembic version table so existing databases are recognised. + alembic_version_table: ClassVar[str] = "alembic_version" + # The tables langflow provisions via create_db_and_tables sanity check. + expected_tables: ClassVar[tuple[str, ...]] = ( + "flow", + "user", + "apikey", + "folder", + "message", + "variable", + "transaction", + "vertex_build", + "job", ) - deadline = time.monotonic() + timeout - while time.monotonic() < deadline: - time.sleep(_MIGRATION_LOCK_POLL_INTERVAL_S) - if conn.execute(sa.text(f"SELECT pg_try_advisory_lock({lock_id})")).scalar(): - logger.info("Acquired migration advisory lock %s after waiting.", lock_id) - return - msg = ( - f"Could not acquire migration advisory lock {lock_id} within " - f"{timeout:.0f}s. Another worker is likely hung mid-migration. " - "Investigate the worker holding the lock or restart the deployment " - "with a single worker so migrations can run cleanly. Override the " - "wait via LANGFLOW_MIGRATION_LOCK_TIMEOUT_S (seconds) if your migration " - "legitimately needs longer." - ) - raise RuntimeError(msg) - - -def _normalize_sync_postgres_url(database_url: str) -> str: - """Return a sync-driver Postgres URL from a possibly async one. - - Strips the ``+asyncpg`` / ``+aiosqlite`` suffix and upgrades the legacy - ``postgres://`` scheme to ``postgresql://`` so :func:`sa.create_engine` - picks the default sync driver. Centralised so the advisory-lock helper and - the table-creation lock path stay in sync with :func:`check_postgresql_version_sync`. - """ - sync_url = database_url - if sync_url.startswith("postgres://"): - sync_url = "postgresql://" + sync_url.split("://", 1)[1] - for async_driver in ("+asyncpg", "+aiosqlite"): - sync_url = sync_url.replace(async_driver, "") - return sync_url - - -@contextmanager -def _postgres_migration_lock(database_url: str): - """Hold a Postgres session-level advisory lock for the duration of the block. - - Workers starting concurrently against a fresh PostgreSQL each call - ``command.upgrade("head")``; without coordination they race on - ``CREATE TYPE`` / ``CREATE TABLE`` and the losers fail with - ``UniqueViolation``. Holding a session-level advisory lock serialises the - upgrade so only one worker mutates the schema at a time; the others wait - here (bounded, with progress logging) and then find the schema already at - head. - - No-op for non-PostgreSQL URLs. SQLite has no advisory locks (and Langflow - runs single-process on it anyway). - """ - if not database_url.startswith(("postgresql", "postgres")): - yield - return - - engine = sa.create_engine(_normalize_sync_postgres_url(database_url)) - try: - with engine.connect() as conn: - logger.debug("Acquiring migration advisory lock %s", _MIGRATION_ADVISORY_LOCK_ID) - _acquire_migration_lock_or_raise(conn, _MIGRATION_ADVISORY_LOCK_ID) - try: - yield - finally: - logger.debug("Releasing migration advisory lock %s", _MIGRATION_ADVISORY_LOCK_ID) - # Session-level locks auto-release on connection close, but - # explicit unlock keeps the connection reusable if alembic - # internals ever hand us one back. - conn.execute(sa.text(f"SELECT pg_advisory_unlock({_MIGRATION_ADVISORY_LOCK_ID})")) - finally: - engine.dispose() - - -def _check_version_row(version_num_str: str, version_str: str) -> None: - """Raise ``UnsupportedPostgreSQLVersionError`` when the version is too old.""" - if int(version_num_str) < MIN_POSTGRESQL_MAJOR_VERSION * 10000: - msg = f"Running PostgreSQL {version_str}. {POSTGRESQL_VERSION_REQUIRED_MESSAGE}" - logger.error(msg) - raise UnsupportedPostgreSQLVersionError(msg) - - -def check_postgresql_version_sync(database_url: str) -> None: - """Pre-flight check: verify PostgreSQL >= 15 using a synchronous connection. - - Call this *before* starting uvicorn/gunicorn so a version mismatch - results in a clean ``sys.exit(1)`` rather than a messy lifespan failure. - Silently returns when the URL is not PostgreSQL. - """ - if not database_url.startswith(("postgresql", "postgres")): - return - - from sqlalchemy import create_engine - - engine = create_engine(_normalize_sync_postgres_url(database_url)) - try: - with engine.connect() as conn: - row = conn.execute(_PG_VERSION_QUERY).fetchone() - _check_version_row(*row) - finally: - engine.dispose() - - -def get_sqlite_database_file_path(database_url: str) -> Path | None: - """Return the on-disk file path for a SQLite URL, or ``None`` when there is none. - - Returns ``None`` for non-SQLite URLs and for in-memory SQLite databases - (``sqlite://`` and ``sqlite:///:memory:``), which have no file on disk. The - returned path is kept exactly as written in the URL (relative paths are *not* - resolved) so callers can report it back to the user verbatim. - """ - if not database_url.startswith("sqlite"): - return None - try: - database = make_url(database_url).database - except Exception: # noqa: BLE001 - defensive: malformed URLs are handled elsewhere - return None - if not database or database == ":memory:": - return None - return Path(database) - - -def check_sqlite_database_path(database_url: str) -> None: - """Fail fast with an actionable message when a SQLite database cannot be opened. - - SQLite does not create intermediate directories, and relative paths in - ``LANGFLOW_DATABASE_URL`` are resolved by SQLAlchemy against the current - working directory at connect time. When the resolved parent directory is - missing the raw ``sqlite3.OperationalError`` ("unable to open database file") - is opaque, so surface where Langflow actually tried to open the database and - how a relative path was resolved. No-op for non-SQLite and in-memory URLs. - - Note: this only improves diagnostics; it does not change which URLs are - accepted nor create any directories. See issue #13634. - """ - db_path = get_sqlite_database_file_path(database_url) - if db_path is None: - return - - resolved = db_path.resolve() - logger.debug(f"Using SQLite database at {resolved}") - - parent = resolved.parent - if parent.exists(): - return - - msg = ( - f"Cannot open the SQLite database at '{resolved}': the parent directory " - f"'{parent}' does not exist, and SQLite does not create intermediate " - f"directories. " - ) - if db_path.is_absolute(): - msg += "Create the directory before starting Langflow, or point LANGFLOW_DATABASE_URL at an existing path." - else: - msg += ( - f"The relative path '{db_path}' from LANGFLOW_DATABASE_URL was resolved against the current working " - f"directory ('{Path.cwd()}'). Set LANGFLOW_DATABASE_URL to an absolute path " - f"(e.g. 'sqlite:///{resolved}'), or create the directory before starting Langflow." - ) - raise ValueError(msg) - - -class DatabaseService(Service): - name = "database_service" - - # Tier 1 infrastructure port. A real engine persists across restarts; SHARED - # (cross-process) is only true for Postgres, so the static class-level - # guarantee is the intersection: {PERSISTENT}. (A backend that wants to - # advertise SHARED can override per deployment.) - tier: ClassVar[Tier] = Tier.INFRASTRUCTURE - capabilities: ClassVar[frozenset[Capability]] = frozenset({Capability.PERSISTENT}) - - def session_scope(self): - """Async write session scope over this service (auto-commit/rollback). - - Part of the Tier 1 database port (Option B): Tier 2 services call this on - their injected ``database_service``. Delegates to the shared helper so - semantics match the module-level ``lfx.services.deps.session_scope``. - """ - from lfx.services.database.session import session_scope_for - - return session_scope_for(self) - - def session_scope_readonly(self): - """Read-only session scope over this service (no commit/rollback).""" - from lfx.services.database.session import session_scope_readonly_for - - return session_scope_readonly_for(self) - - def __init__(self, settings_service: SettingsService): - self._logged_pragma = False - self.settings_service = settings_service - if settings_service.settings.database_url is None: - msg = "No database URL provided" - raise ValueError(msg) - self.database_url: str = settings_service.settings.database_url - - configure_windows_postgres_event_loop(source="database_service") - - self._sanitize_database_url() - - # This file is in langflow.services.database.manager.py - # the ini is in langflow + def _resolve_script_location(self) -> Path: + """Point the migration runner at langflow's own alembic tree.""" langflow_dir = Path(__file__).parent.parent.parent - self.script_location = langflow_dir / "alembic" - self.alembic_cfg_path = langflow_dir / "alembic.ini" - - # register the event listener for sqlite as part of this class. - # Using decorator will make the method not able to use self - event.listen(Engine, "connect", self.on_connection) - if self.settings_service.settings.database_connection_retry: - self.engine = self._create_engine_with_retry() - else: - self.engine = self._create_engine() - - # Create async session maker for efficient session creation - # This is the recommended SQLAlchemy 2.0+ pattern - # IMPORTANT: Must use SQLModel's AsyncSession (not SQLAlchemy's) for exec() method - self.async_session_maker = async_sessionmaker( - self.engine, - class_=SQLModelAsyncSession, # SQLModel's AsyncSession with exec() support - expire_on_commit=False, - ) - - # Check if Alembic should log to stdout or a file. - # If file, check if the provided path is absolute, cross-platform. - alembic_log_file = self.settings_service.settings.alembic_log_file - self.alembic_log_to_stdout = self.settings_service.settings.alembic_log_to_stdout - if self.alembic_log_to_stdout: - self.alembic_log_path = None - elif Path(alembic_log_file).is_absolute(): - self.alembic_log_path = Path(alembic_log_file) - else: - # Resolve relative log paths against the writable runtime config - # directory, not the installed package directory. The package dir is - # read-only in hardened deployments (non-root containers, read-only - # root filesystems, Kubernetes), where writing into it raises OSError - # and crashes startup. config_dir is always writable (it defaults to - # platformdirs' user cache dir and is created on startup). - config_dir = getattr(self.settings_service.settings, "config_dir", None) - base_dir = Path(config_dir) if config_dir else langflow_dir - self.alembic_log_path = base_dir / alembic_log_file - - async def initialize_alembic_log_file(self): - log_path = self.alembic_log_path - if self.alembic_log_to_stdout or log_path is None: - return - # Ensure the directory and file for the alembic log file exists. The - # migration log is diagnostic-only, so a read-only filesystem (hardened - # containers / Kubernetes) must never abort startup: warn and move on. - try: - await anyio.Path(log_path.parent).mkdir(parents=True, exist_ok=True) - await anyio.Path(log_path).touch(exist_ok=True) - except OSError as exc: - await logger.awarning( - f"Could not initialize the Alembic migration log at '{log_path}' ({exc}). " - "Migration output falls back to stdout. Set LANGFLOW_ALEMBIC_LOG_FILE to a writable path " - "or LANGFLOW_ALEMBIC_LOG_TO_STDOUT=true to silence this warning." - ) - - def reload_engine(self) -> None: - self._sanitize_database_url() - if self.settings_service.settings.database_connection_retry: - self.engine = self._create_engine_with_retry() - else: - self.engine = self._create_engine() - - self.async_session_maker = async_sessionmaker( - self.engine, - class_=SQLModelAsyncSession, - expire_on_commit=False, - ) - - def _sanitize_database_url(self): - """Create the engine for the database.""" - url_components = self.database_url.split("://", maxsplit=1) - - driver = url_components[0] - - if driver == "sqlite": - driver = "sqlite+aiosqlite" - elif driver in {"postgresql", "postgres"}: - if driver == "postgres": - logger.warning( - "The postgres dialect in the database URL is deprecated. " - "Use postgresql instead. " - "To avoid this warning, update the database URL." - ) - driver = "postgresql+psycopg" - - self.database_url = f"{driver}://{url_components[1]}" - - def _build_connection_kwargs(self): - """Build connection kwargs by merging deprecated settings with db_connection_settings. - - Returns: - dict: Connection kwargs with deprecated settings overriding db_connection_settings - """ - settings = self.settings_service.settings - # Start with db_connection_settings as base - connection_kwargs = settings.db_connection_settings.copy() - - # Override individual settings if explicitly set - if "pool_size" in settings.model_fields_set: - logger.warning("pool_size is deprecated. Use db_connection_settings['pool_size'] instead.") - connection_kwargs["pool_size"] = settings.pool_size - if "max_overflow" in settings.model_fields_set: - logger.warning("max_overflow is deprecated. Use db_connection_settings['max_overflow'] instead.") - connection_kwargs["max_overflow"] = settings.max_overflow - - return connection_kwargs - - def _create_engine(self) -> AsyncEngine: - # Get connection settings from config, with defaults if not specified - # if the user specifies an empty dict, we allow it. - kwargs = self._build_connection_kwargs() - - poolclass_key = kwargs.get("poolclass") - if poolclass_key is not None: - pool_class = getattr(sa.pool, poolclass_key, None) - if pool_class and issubclass(pool_class, sa.pool.Pool): - logger.debug(f"Using poolclass: {poolclass_key}.") - kwargs["poolclass"] = pool_class - else: - logger.error(f"Invalid poolclass '{poolclass_key}' specified. Using default pool class.") - kwargs.pop("poolclass", None) - - return create_async_engine( - self.database_url, - connect_args=self._get_connect_args(), - **kwargs, - ) - - @retry(wait=wait_fixed(2), stop=stop_after_attempt(10)) - def _create_engine_with_retry(self) -> AsyncEngine: - """Create the engine for the database with retry logic.""" - return self._create_engine() - - def _get_connect_args(self): - settings = self.settings_service.settings - - if settings.db_driver_connection_settings is not None: - return settings.db_driver_connection_settings - - if settings.database_url and settings.database_url.startswith("sqlite"): - return { - "check_same_thread": False, - "timeout": settings.db_connect_timeout, - } - # For PostgreSQL, set the timezone to UTC - if settings.database_url and settings.database_url.startswith(("postgresql", "postgres")): - return {"options": "-c timezone=utc"} - return {} - - def on_connection(self, dbapi_connection, _connection_record) -> None: - if isinstance(dbapi_connection, sqlite3.Connection | dialect_sqlite.aiosqlite.AsyncAdapt_aiosqlite_connection): - pragmas: dict = self.settings_service.settings.sqlite_pragmas or {} - pragmas_list = [] - for key, val in pragmas.items(): - pragmas_list.append(f"PRAGMA {key} = {val}") - if not self._logged_pragma: - logger.debug(f"sqlite connection, setting pragmas: {pragmas_list}") - self._logged_pragma = True - if pragmas_list: - cursor = dbapi_connection.cursor() - try: - for pragma in pragmas_list: - try: - cursor.execute(pragma) - except OperationalError: - logger.exception(f"Failed to set PRAGMA {pragma}") - except GeneratorExit: - logger.error(f"Failed to set PRAGMA {pragma}") - finally: - cursor.close() - - @asynccontextmanager - async def _with_session(self): - """Internal method to create a session. DO NOT USE DIRECTLY. - - Use session_scope() for write operations or session_scope_readonly() for read operations. - This method does not handle commits - it only provides a raw session. - """ - if self.settings_service.settings.use_noop_database: - yield NoopSession() - else: - # Use async_session_maker - the recommended SQLAlchemy 2.0+ pattern - # Provides efficient session creation and proper connection pooling - async with self.async_session_maker() as session: - yield session - - async def ensure_postgresql_version(self) -> None: - """If the database is PostgreSQL, ensure it is version 15 or higher. - - Langflow's schema uses UNIQUE NULLS DISTINCT, which is only supported in PostgreSQL 15+. - Logs the message and raises UnsupportedPostgreSQLVersionError if the version is too old. - """ - if not self.database_url.startswith(("postgresql", "postgres")): - return - if self.settings_service.settings.use_noop_database: - return - async with session_scope() as session: - result = await session.execute(_PG_VERSION_QUERY) - version_num_str, version_str = result.fetchone() - # Raise AFTER session_scope exits so session_scope doesn't log a - # noisy "An error occurred during the session scope." traceback. - _check_version_row(version_num_str, version_str) + return langflow_dir / "alembic" async def assign_orphaned_flows_to_superuser(self) -> None: """Assign orphaned flows to the default superuser when auto login is enabled.""" @@ -613,123 +193,10 @@ async def check_schema_health(self) -> None: async with self.engine.begin() as conn: await conn.run_sync(self._check_schema_health) - @staticmethod - def init_alembic(alembic_cfg) -> None: - logger.info("Initializing alembic") - command.ensure_version(alembic_cfg) - # alembic_cfg.attributes["connection"].commit() - command.upgrade(alembic_cfg, "head") - - def _open_alembic_log_buffer(self): - """Open the Alembic migration log for writing, falling back to stdout. - - The migration log is diagnostic-only output. If the target path cannot - be written -- e.g. the installed package directory or the root - filesystem is read-only, as in hardened container/Kubernetes deployments - (non-root user or read-only root filesystem) -- startup must not abort. - Fall back to stdout rather than letting OSError propagate through the - FastAPI lifespan. Returns a context manager yielding the buffer Alembic - writes its output to. - """ - log_path = self.alembic_log_path - if self.alembic_log_to_stdout or log_path is None: - return nullcontext(sys.stdout) - try: - # _run_migrations can run before initialize_alembic_log_file(), so - # make sure the parent directory exists before opening for writing. - log_path.parent.mkdir(parents=True, exist_ok=True) - return log_path.open("w", encoding="utf-8") - except OSError as exc: - logger.warning( - f"Could not open the Alembic migration log at '{log_path}' ({exc}). " - "Falling back to stdout. Set LANGFLOW_ALEMBIC_LOG_FILE to a writable path " - "or LANGFLOW_ALEMBIC_LOG_TO_STDOUT=true to silence this warning." - ) - return nullcontext(sys.stdout) - - def _run_migrations(self, should_initialize_alembic, fix) -> None: - # First we need to check if alembic has been initialized - # If not, we need to initialize it - # if not self.script_location.exists(): # this is not the correct way to check if alembic has been initialized - # We need to check if the alembic_version table exists - # if not, we need to initialize alembic - # stdout should be something like sys.stdout - # which is a buffer - # I don't want to output anything - # subprocess.DEVNULL is an int - buffer_context = self._open_alembic_log_buffer() - # The advisory lock serialises concurrent migration runs across workers - # so they do not race on CREATE TYPE / CREATE TABLE against a fresh PG. - with _postgres_migration_lock(self.database_url), buffer_context as buffer: - alembic_cfg = Config(stdout=buffer) - # alembic_cfg.attributes["connection"] = session - alembic_cfg.set_main_option("script_location", str(self.script_location)) - alembic_cfg.set_main_option("sqlalchemy.url", self.database_url.replace("%", "%%")) - - if should_initialize_alembic: - try: - self.init_alembic(alembic_cfg) - except Exception as exc: - msg = f"Error initializing alembic: {exc}" - logger.exception(msg) - raise RuntimeError(msg) from exc - else: - logger.debug("Alembic initialized") - - try: - buffer.write(f"{datetime.now(tz=timezone.utc).astimezone().isoformat()}: Checking migrations\n") - command.check(alembic_cfg) - except Exception as exc: # noqa: BLE001 - logger.debug(f"Error checking migrations: {exc}") - if isinstance(exc, util.exc.CommandError | util.exc.AutogenerateDiffsDetected): - command.upgrade(alembic_cfg, "head") - time.sleep(3) - - try: - buffer.write(f"{datetime.now(tz=timezone.utc).astimezone()}: Checking migrations\n") - command.check(alembic_cfg) - except util.exc.AutogenerateDiffsDetected as exc: - logger.exception("Error checking migrations") - if not fix: - msg = f"There's a mismatch between the models and the database.\n{exc}" - raise RuntimeError(msg) from exc - - if fix: - self.try_downgrade_upgrade_until_success(alembic_cfg) - - async def run_migrations(self, *, fix=False) -> None: - should_initialize_alembic = False - async with session_scope() as session: - # If the table does not exist it throws an error - # so we need to catch it - try: - await session.exec(text("SELECT * FROM alembic_version")) - except Exception: # noqa: BLE001 - await logger.adebug("Alembic not initialized") - should_initialize_alembic = True - await asyncio.to_thread(self._run_migrations, should_initialize_alembic, fix) - - @staticmethod - def try_downgrade_upgrade_until_success(alembic_cfg, retries=5) -> None: - # Try -1 then head, if it fails, try -2 then head, etc. - # until we reach the number of retries - for i in range(1, retries + 1): - try: - command.check(alembic_cfg) - break - except util.exc.AutogenerateDiffsDetected: - # downgrade to base and upgrade again - logger.warning("AutogenerateDiffsDetected") - command.downgrade(alembic_cfg, f"-{i}") - # wait for the database to be ready - time.sleep(3) - command.upgrade(alembic_cfg, "head") - async def run_migrations_test(self): # This method is used for testing purposes only # We will check that all models are in the database # and that the database is up to date with all columns - # get all models that are subclasses of SQLModel sql_models = [ model for model in models.__dict__.values() if isinstance(model, type) and issubclass(model, SQLModel) ] @@ -764,9 +231,9 @@ def check_table(connection, model): @staticmethod def _create_db_and_tables(connection) -> None: - from sqlalchemy import inspect + from sqlalchemy import inspect as sa_inspect - inspector = inspect(connection) + inspector = sa_inspect(connection) table_names = inspector.get_table_names() current_tables = [ "flow", @@ -789,7 +256,7 @@ def _create_db_and_tables(connection) -> None: for table in SQLModel.metadata.sorted_tables: try: table.create(connection, checkfirst=True) - except OperationalError as oe: + except sa.exc.OperationalError as oe: logger.warning(f"Table {table} already exists, skipping. Exception: {oe}") except Exception as exc: msg = f"Error creating table {table}" @@ -797,7 +264,7 @@ def _create_db_and_tables(connection) -> None: raise RuntimeError(msg) from exc # Now check if the required tables exist, if not, something went wrong. - inspector = inspect(connection) + inspector = sa_inspect(connection) table_names = inspector.get_table_names() for table in current_tables: if table not in table_names: @@ -808,40 +275,6 @@ def _create_db_and_tables(connection) -> None: logger.debug("Database and tables created successfully") - @retry(wait=wait_fixed(2), stop=stop_after_attempt(10)) - async def create_db_and_tables_with_retry(self) -> None: - await self.create_db_and_tables() - - async def create_db_and_tables(self) -> None: - if not self.database_url.startswith(("postgresql", "postgres")): - # SQLite / non-PG: original async path; advisory lock does not apply. - async with self.engine.begin() as conn: - await conn.run_sync(self._create_db_and_tables) - return - - # Postgres: serialise CREATE TYPE / CREATE TABLE across workers under - # the same advisory lock that protects run_migrations. Without this, - # concurrent workers booting against a fresh database race on - # ``table.create(checkfirst=True)`` and the losers fail with - # ``UniqueViolation`` on ``pg_type_typname_nsp_index``. The lock is - # acquired synchronously; run in a worker thread so a contended-lock - # poll does not block the event loop. - await asyncio.to_thread(self._create_db_and_tables_with_lock) - - def _create_db_and_tables_with_lock(self) -> None: - """Postgres path: hold the migration advisory lock for the DDL. - - Opens its own sync engine so the DDL runs on the same driver the lock - uses; the application's async engine is unaffected. - """ - with _postgres_migration_lock(self.database_url): - sync_engine = sa.create_engine(_normalize_sync_postgres_url(self.database_url)) - try: - with sync_engine.begin() as conn: - self._create_db_and_tables(conn) - finally: - sync_engine.dispose() - async def teardown(self) -> None: await logger.adebug("Tearing down database") try: diff --git a/src/backend/tests/unit/test_database_windows_postgres_integration.py b/src/backend/tests/unit/test_database_windows_postgres_integration.py index acae183e53a8..510cec72396a 100644 --- a/src/backend/tests/unit/test_database_windows_postgres_integration.py +++ b/src/backend/tests/unit/test_database_windows_postgres_integration.py @@ -2,6 +2,11 @@ Tests that the database service properly handles event loop configuration across different platforms and database types. + +The engine/session core moved into ``lfx.services.database.service`` (langflow's +``DatabaseService`` is now a thin subclass), so the engine and Windows-helper +symbols are patched on the lfx module -- that is the namespace the inherited +``__init__`` actually resolves them from. """ import asyncio @@ -30,8 +35,8 @@ def mock_settings_service(self): @patch("platform.system") @patch.dict(os.environ, {"LANGFLOW_DATABASE_URL": "postgresql://user:pass@localhost/db"}, clear=True) - @patch("langflow.services.database.service.create_async_engine") - @patch("langflow.services.database.service.configure_windows_postgres_event_loop") + @patch("lfx.services.database.service.create_async_engine") + @patch("lfx.services.database.service.configure_windows_postgres_event_loop") def test_windows_postgresql_configures_event_loop( self, mock_configure, mock_create_engine, mock_platform, mock_settings_service ): @@ -46,7 +51,7 @@ def test_windows_postgresql_configures_event_loop( @patch("platform.system") @patch.dict(os.environ, {}, clear=True) - @patch("langflow.services.database.service.create_async_engine") + @patch("lfx.services.database.service.create_async_engine") def test_linux_postgresql_no_event_loop_change(self, mock_create_engine, mock_platform, mock_settings_service): """Test that Linux + PostgreSQL doesn't change event loop.""" mock_platform.return_value = "Linux" @@ -61,7 +66,7 @@ def test_linux_postgresql_no_event_loop_change(self, mock_create_engine, mock_pl @patch("platform.system") @patch.dict(os.environ, {}, clear=True) - @patch("langflow.services.database.service.create_async_engine") + @patch("lfx.services.database.service.create_async_engine") def test_macos_postgresql_no_event_loop_change(self, mock_create_engine, mock_platform, mock_settings_service): """Test that macOS + PostgreSQL doesn't change event loop.""" mock_platform.return_value = "Darwin" @@ -76,8 +81,8 @@ def test_macos_postgresql_no_event_loop_change(self, mock_create_engine, mock_pl @patch("platform.system") @patch.dict(os.environ, {}, clear=True) - @patch("langflow.services.database.service.create_async_engine") - @patch("langflow.services.database.service.configure_windows_postgres_event_loop") + @patch("lfx.services.database.service.create_async_engine") + @patch("lfx.services.database.service.configure_windows_postgres_event_loop") def test_windows_sqlite_no_event_loop_change( self, mock_configure, mock_create_engine, mock_platform, mock_settings_service ): @@ -98,7 +103,7 @@ def test_database_url_sanitization(self, mock_settings_service): ("postgres://user:pass@localhost/db", "postgresql+psycopg://user:pass@localhost/db"), ] - with patch("langflow.services.database.service.create_async_engine") as mock_create_engine: + with patch("lfx.services.database.service.create_async_engine") as mock_create_engine: mock_create_engine.return_value = MagicMock() for input_url, expected_url in test_cases: @@ -113,7 +118,7 @@ def test_docker_environment_compatibility(self, mock_platform, mock_settings_ser os.environ["DOCKER_CONTAINER"] = "true" mock_settings_service.settings.database_url = "postgresql://postgres:5432/langflow" - with patch("langflow.services.database.service.create_async_engine") as mock_create_engine: + with patch("lfx.services.database.service.create_async_engine") as mock_create_engine: mock_create_engine.return_value = MagicMock() # Should not raise any errors @@ -125,7 +130,7 @@ async def test_async_operations_work_after_configuration(self, mock_settings_ser """Test that async operations work correctly after event loop configuration.""" mock_settings_service.settings.database_url = "sqlite:///test.db" - with patch("langflow.services.database.service.create_async_engine") as mock_create_engine: + with patch("lfx.services.database.service.create_async_engine") as mock_create_engine: mock_engine = MagicMock() mock_create_engine.return_value = mock_engine diff --git a/src/lfx/PLUGGABLE_SERVICES.md b/src/lfx/PLUGGABLE_SERVICES.md index b213a0a1fa6c..31ca781dad0f 100644 --- a/src/lfx/PLUGGABLE_SERVICES.md +++ b/src/lfx/PLUGGABLE_SERVICES.md @@ -128,6 +128,61 @@ the builder and the production worker-plane; only the Tier 1 `DatabaseService` b changes. That is the target: **overriding is the exception (auth, RBAC, genuinely different backends), not how langflow consumes lfx.** +### Reference example: the database (Tier 1) and its migration stream + +`lfx.services.database` is the Tier 1 worked example — the engine/session/migration +core a bare `lfx serve` needs to persist in production without langflow: + +- `service.py` — `NoopDatabaseService` (the ephemeral default; `capabilities=()`) and the + real `DatabaseService` (`{PERSISTENT}`): a pooled async engine plus an alembic runner. +- `migrations/` — lfx's **own** alembic lineage (version table `lfx_alembic_version`), + scoped by `include_name`/`include_object` to the execution-history tables lfx owns + (`message`, `transaction`, `vertex_build`). + +**How langflow consumes it:** langflow *does* subclass here — the allowed exception. Its +`DatabaseService(lfx…DatabaseService)` keeps the inherited engine/session/migration +mechanism and overrides only its migration-stream identity (version table +`alembic_version`, `script_location` → `langflow/alembic`) plus domain bootstrap (superuser +assignment, schema-health). The subclass is justified because langflow's migration lineage +and domain policy genuinely differ from a bare `lfx serve`. + +#### Two-stream migration model (no divergence) + +lfx and langflow each own an **independent** alembic lineage against a **distinct** version +table. A given physical database is provisioned by exactly one stream — the langflow editor +DB by langflow's lineage (full schema), the scaled `lfx serve` DB by lfx's (execution +tables only). + +This does **not** cause schema divergence, because the model classes are the single source +of truth (they live once, in `lfx.services.database.models`, imported by both streams). The +streams can only differ in *staleness*, which is mechanically caught: `lfx db check` (and +langflow's equivalent) run alembic's autogenerate-diff per stream in CI and fail the moment +a stream lags the model. The `NAMING_CONVENTION` is byte-identical across both env modules +so shared tables get identically-named constraints. Isolation works via the `include_*` +filters: a langflow-only model is invisible to lfx's stream; an lfx-core model change is +visible to both and so needs a revision in each (almost always the case, since lfx is nested +in langflow). + +The end-state (full lfx ownership) is then a non-breaking flip: langflow's env excludes the +lfx-core tables and delegates them to lfx's stream. Today's separate-lineage model is a +strict subset of that, so nothing is painted into a corner. + +#### Hydrating Tier 1 at boot + +*Which implementation* and *how it connects* are separate inputs: + +- **Wiring** — `lfx.toml` `[services]` maps each `ServiceType` to a class + (`database_service = "lfx.services.database.service:DatabaseService"`). Discovered from + `$LANGFLOW_CONFIG_DIR` at boot; precedence is **config file > entry points > defaults**. +- **Connection** — environment (`LANGFLOW_DATABASE_URL`, pool sizes). Never in `lfx.toml`. + +With **no** `lfx.toml`, bare lfx stays ephemeral (`NoopDatabaseService` + +`InMemoryMemoryService`) — what `lfx run` wants. Adding the wiring flips the same process to +persistent Postgres + DB-backed memory with no code change: the editor↔production parity +goal. Migrations are **explicit** (`lfx db upgrade`); `lfx serve` verifies the schema is at +head and refuses to start otherwise. See [`deploy/`](deploy/) for ConfigMap/Secret/Knative +stubs and the full hydration walkthrough. + ## Adapter Registries (Service-Scoped Plugin Registries) LFX also supports **adapter registries** -- collections of swappable implementations that share the same protocol. diff --git a/src/lfx/deploy/README.md b/src/lfx/deploy/README.md new file mode 100644 index 000000000000..44cd36797d61 --- /dev/null +++ b/src/lfx/deploy/README.md @@ -0,0 +1,65 @@ +# Deploying lfx as a configurable service + +This directory shows how to hydrate lfx's **Tier 1** (infrastructure) and +**Tier 2** (composed) services for a production `lfx serve` deployment. It is +scoped to the two services extracted into lfx so far — the database and the +DB-backed chat memory — and the same pattern extends to the rest as they land. + +## The two hydration inputs + +lfx separates *which implementation* from *how it connects*: + +| Input | Mechanism | What it carries | Example | +|-------|-----------|-----------------|---------| +| **Wiring** | `lfx.toml` `[services]` | which class backs each `ServiceType` | `database_service = "…:DatabaseService"` | +| **Connection / tunables** | environment (`LANGFLOW_*`) | URLs, credentials, pool sizes | `LANGFLOW_DATABASE_URL=postgresql://…` | + +At boot the service manager discovers `lfx.toml` in `$LANGFLOW_CONFIG_DIR` and +selects those implementations; each service then reads its own connection +settings from the environment. Precedence is **`lfx.toml` > entry-point plugins +> built-in defaults**. + +With **no `lfx.toml`**, bare lfx stays ephemeral — `NoopDatabaseService` + +`InMemoryMemoryService` — which is exactly what `lfx run` wants. Adding the +`lfx.toml` here flips the same process to persistent Postgres + DB-backed memory +without any code change. That is the editor↔production parity goal: the same +engine code, wired differently. + +## Files + +- [`lfx.toml`](lfx.toml) — the production wiring (database + memory). +- [`k8s/configmap.yaml`](k8s/configmap.yaml) — mounts `lfx.toml` at `/config` and + carries non-secret tunables. +- [`k8s/secret.example.yaml`](k8s/secret.example.yaml) — shape of the Secret + holding `LANGFLOW_DATABASE_URL` (credentials). +- [`k8s/knative-service.yaml`](k8s/knative-service.yaml) — a scale-to-zero + gateway that mounts both and runs `lfx db upgrade` as an initContainer. + +## Migrations are explicit + +lfx does **not** migrate implicitly on `serve`. Provision the schema first: + +```bash +export LANGFLOW_DATABASE_URL='postgresql://user:pass@host:5432/lfx' +lfx db upgrade # apply lfx's migration stream to head +lfx db current # confirm the head revision +``` + +In Kubernetes this is the initContainer (or a one-shot Job). lfx's migration +stream is **its own** alembic lineage (version table `lfx_alembic_version`), +independent of langflow's — see the two-stream model in +[`../PLUGGABLE_SERVICES.md`](../PLUGGABLE_SERVICES.md). A given physical database +is owned by exactly one stream: the langflow editor DB by langflow's lineage, the +scaled `lfx serve` DB by lfx's. + +## Quick local smoke test (SQLite) + +```bash +export LANGFLOW_CONFIG_DIR="$PWD/deploy" # find lfx.toml +export LANGFLOW_DATABASE_URL="sqlite:///$PWD/lfx.db" +lfx db upgrade # creates message/transaction/vertex_build +lfx serve path/to/flow.json # persistent chat memory over SQLite +``` + +Point `LANGFLOW_DATABASE_URL` at Postgres and the same commands provision and +serve against Postgres instead. diff --git a/src/lfx/deploy/k8s/configmap.yaml b/src/lfx/deploy/k8s/configmap.yaml new file mode 100644 index 000000000000..67c14cb7cb84 --- /dev/null +++ b/src/lfx/deploy/k8s/configmap.yaml @@ -0,0 +1,42 @@ +# Non-secret hydration for an lfx deployment. +# +# Two things live here: +# 1. lfx.toml -- the service wiring (which Tier 1/Tier 2 impls to select). +# 2. Plain env -- non-secret tunables (pool sizes, log routing, config dir). +# +# Secrets (LANGFLOW_DATABASE_URL with credentials, API keys) go in a Secret -- +# see secret.example.yaml. Mount this ConfigMap's lfx.toml into +# $LANGFLOW_CONFIG_DIR so the service manager discovers it at boot. +apiVersion: v1 +kind: ConfigMap +metadata: + name: lfx-config + labels: + app: lfx +data: + # Rendered to $LANGFLOW_CONFIG_DIR/lfx.toml via a volumeMount (see the + # knative-service.yaml example). Keep in sync with deploy/lfx.toml. + lfx.toml: | + [services] + database_service = "lfx.services.database.service:DatabaseService" + memory_service = "lfx.services.memory.database:DatabaseMemoryService" + + # Where lfx looks for lfx.toml. The ConfigMap volume is mounted here. + LANGFLOW_CONFIG_DIR: "/config" + + # Send alembic/migration output to stdout (container-friendly; the package + # dir is read-only under a non-root user). + LANGFLOW_ALEMBIC_LOG_TO_STDOUT: "true" + + # --- Tier 1 database tunables (optional; shown with their defaults) --- + # Connection pool sizing for the async engine. Raise for higher concurrency. + # (The engine also honors the richer db_connection_settings dict in settings.) + # These are read by lfx.services.settings.groups.database.DatabaseSettings. + # LANGFLOW_POOL_SIZE: "20" + # LANGFLOW_MAX_OVERFLOW: "30" + # LANGFLOW_DB_CONNECT_TIMEOUT: "30" + + # Advisory-lock namespace so concurrent workers coordinate migrations against + # a shared Postgres. Set to a per-deployment value when multiple deployments + # share one database. + # LANGFLOW_MIGRATION_LOCK_NAMESPACE: "lfx-prod" diff --git a/src/lfx/deploy/k8s/knative-service.yaml b/src/lfx/deploy/k8s/knative-service.yaml new file mode 100644 index 000000000000..e94f2a5ef7fc --- /dev/null +++ b/src/lfx/deploy/k8s/knative-service.yaml @@ -0,0 +1,63 @@ +# Knative Service stub for the lfx serve gateway. +# +# Ties the hydration together: +# * the ConfigMap's lfx.toml is mounted at /config (= LANGFLOW_CONFIG_DIR), so +# the service manager discovers the Tier 1/Tier 2 wiring at boot; +# * the Secret supplies LANGFLOW_DATABASE_URL; +# * an initContainer runs `lfx db upgrade` BEFORE the serve container starts. +# +# The initContainer is deliberate: lfx uses an explicit-migration policy (it does +# NOT migrate implicitly on serve). Provisioning the schema in an init step keeps +# migrations off the request path and out of a scale-to-zero cold start, and the +# Postgres advisory lock makes concurrent replicas safe. (Alternatively run +# `lfx db upgrade` as a one-shot Job in your release pipeline and drop the +# initContainer.) +# +# Scope note: this is the gateway role. A dedicated worker pool (executing queued +# payloads) is a separate Service with the same image and a different command, +# added once the Tier 1 queue service lands. +apiVersion: serving.knative.dev/v1 +kind: Service +metadata: + name: lfx-serve + labels: + app: lfx +spec: + template: + metadata: + annotations: + autoscaling.knative.dev/minScale: "0" # scale to zero when idle + autoscaling.knative.dev/maxScale: "10" + spec: + initContainers: + - name: lfx-db-upgrade + image: ghcr.io/langflow-ai/lfx:latest # replace with the -prod (driver-baked) tag + command: ["lfx", "db", "upgrade"] + envFrom: + - secretRef: + name: lfx-secrets + env: + - name: LANGFLOW_ALEMBIC_LOG_TO_STDOUT + value: "true" + containers: + - name: lfx-serve + image: ghcr.io/langflow-ai/lfx:latest # replace with the -prod (driver-baked) tag + args: ["serve", "/flows"] # serve the mounted flow(s) + ports: + - containerPort: 8000 + envFrom: + - configMapRef: + name: lfx-config + - secretRef: + name: lfx-secrets + volumeMounts: + - name: lfx-config + mountPath: /config + readOnly: true + volumes: + - name: lfx-config + configMap: + name: lfx-config + items: + - key: lfx.toml + path: lfx.toml diff --git a/src/lfx/deploy/k8s/secret.example.yaml b/src/lfx/deploy/k8s/secret.example.yaml new file mode 100644 index 000000000000..59757438beed --- /dev/null +++ b/src/lfx/deploy/k8s/secret.example.yaml @@ -0,0 +1,25 @@ +# Secret hydration for an lfx deployment (EXAMPLE -- do not commit real values). +# +# The database connection string (with credentials) and any provider API keys +# are secrets and must never live in the ConfigMap or lfx.toml. Create the real +# Secret out of band, e.g.: +# +# kubectl create secret generic lfx-secrets \ +# --from-literal=LANGFLOW_DATABASE_URL='postgresql://user:pass@pg-host:5432/lfx' +# +# ...or manage it through your secrets operator (External Secrets, Sealed +# Secrets, Vault). This file only documents the shape. +apiVersion: v1 +kind: Secret +metadata: + name: lfx-secrets + labels: + app: lfx +type: Opaque +stringData: + # Tier 1 database connection. Async driver is applied automatically + # (postgresql -> postgresql+psycopg, sqlite -> sqlite+aiosqlite). + LANGFLOW_DATABASE_URL: "postgresql://REPLACE_USER:REPLACE_PASSWORD@REPLACE_HOST:5432/lfx" + + # The serve API key clients present as x-api-key (if you front flows with one). + # LANGFLOW_SERVE_API_KEY: "REPLACE_ME" diff --git a/src/lfx/deploy/lfx.toml b/src/lfx/deploy/lfx.toml new file mode 100644 index 000000000000..5948e9370445 --- /dev/null +++ b/src/lfx/deploy/lfx.toml @@ -0,0 +1,22 @@ +# lfx production service wiring (Tier 1 + Tier 2) +# +# Drop this file at $LANGFLOW_CONFIG_DIR/lfx.toml (in Kubernetes, mount it from a +# ConfigMap -- see k8s/configmap.yaml). At boot, lfx's service manager discovers +# this file and selects these implementations; connection strings and secrets +# come from the environment (never put them here). +# +# Precedence: this file > entry-point plugins > lfx built-in defaults. +# With NO lfx.toml present, bare lfx stays ephemeral (NoopDatabaseService + +# InMemoryMemoryService), which is what `lfx run` wants. +# +# Scope note: this PoC wires the two services that have been migrated into lfx -- +# the Tier 1 database and the Tier 2 DB-backed chat memory. Cache, storage, +# queue, etc. will be added here as each is extracted in later PRs. + +[services] +# Tier 1 -- pooled async engine + migration runner. Reads LANGFLOW_DATABASE_URL. +database_service = "lfx.services.database.service:DatabaseService" + +# Tier 2 -- DB-backed chat memory. The manager injects the database_service +# above (Option B) so writes/reads go through the Tier 1 engine. +memory_service = "lfx.services.memory.database:DatabaseMemoryService" diff --git a/src/lfx/pyproject.toml b/src/lfx/pyproject.toml index 9957fdbdcca9..3c2b57d38735 100644 --- a/src/lfx/pyproject.toml +++ b/src/lfx/pyproject.toml @@ -54,6 +54,16 @@ dependencies = [ # schema for rows produced during graph execution (message / transaction / # vertex_build); langflow re-exports them and keeps the alembic migrations. "sqlmodel~=0.0.37", + # lfx owns its own alembic migration stream (lfx.services.database.migrations) + # so bare ``lfx serve`` can provision its execution-history schema in + # production without langflow present. See PLUGGABLE_SERVICES.md + # (two-stream migration model). Runtime dep because ``lfx db upgrade`` and the + # DatabaseService migration runner need it, not just tests. + "alembic>=1.13.0,<2.0.0", + # Engine-creation retry in the Tier 1 DatabaseService. Already present + # transitively via langchain; declared here so the extracted service does not + # rely on a transitive import. + "tenacity>=8.0.0,<10.0.0", "ag-ui-protocol>=0.1.10", "langflow-sdk>=0.3.0", "markitdown>=0.1.4,<2.0.0", diff --git a/src/lfx/src/lfx/__main__.py b/src/lfx/src/lfx/__main__.py index baf819bbe1ce..c484d2e01bf5 100644 --- a/src/lfx/src/lfx/__main__.py +++ b/src/lfx/src/lfx/__main__.py @@ -5,6 +5,7 @@ import typer from lfx.cli._authoring_commands import register as _register_authoring +from lfx.cli._db_commands import register as _register_db from lfx.cli._extension_commands import register as _register_extension from lfx.cli._prewarm_commands import register as _register_prewarm from lfx.cli._remote_commands import register as _register_remote @@ -48,6 +49,7 @@ def _app_callback( _register_extension(app) _register_running(app) _register_remote(app) +_register_db(app) def main(): diff --git a/src/lfx/src/lfx/cli/_db_commands.py b/src/lfx/src/lfx/cli/_db_commands.py new file mode 100644 index 000000000000..0b85340b647b --- /dev/null +++ b/src/lfx/src/lfx/cli/_db_commands.py @@ -0,0 +1,121 @@ +"""``lfx db`` -- drive lfx's own migration stream. + +lfx does not migrate implicitly on ``serve`` (explicit-only policy): operators +provision the schema with these commands (or an init container / job) first. +They drive alembic directly against lfx's migration tree +(``lfx.services.database.migrations``, version table ``lfx_alembic_version``), +reading the database URL from settings (``LANGFLOW_DATABASE_URL``). +""" + +from __future__ import annotations + +import typer +from rich.console import Console + +db_app = typer.Typer( + name="db", + help="Manage lfx's database schema (its own migration stream).", + no_args_is_help=True, +) + +_console = Console() +_err_console = Console(stderr=True) + + +def _config(stdout=None): + """Build the alembic Config for lfx's stream from current settings.""" + from lfx.services.database.service import DatabaseService + from lfx.services.deps import get_settings_service + + settings_service = get_settings_service() + if settings_service is None: + _err_console.print("[bold red]Error:[/bold red] settings service unavailable; cannot resolve database URL.") + raise typer.Exit(1) + try: + return DatabaseService.make_cli_config(settings_service, stdout=stdout) + except ValueError as exc: + _err_console.print(f"[bold red]Error:[/bold red] {exc}") + raise typer.Exit(1) from exc + + +@db_app.command(name="upgrade") +def upgrade(revision: str = typer.Argument("head", help="Target revision (default: head).")) -> None: + """Apply migrations up to a revision (creates the schema on a fresh database).""" + from alembic import command + + cfg = _config() + try: + command.upgrade(cfg, revision) + except Exception as exc: + _err_console.print(f"[bold red]Migration failed:[/bold red] {exc}") + raise typer.Exit(1) from exc + _console.print(f"[bold green]OK[/bold green] schema upgraded to {revision!r}.") + + +@db_app.command(name="downgrade") +def downgrade(revision: str = typer.Argument(..., help="Target revision, e.g. '-1' or a revision id.")) -> None: + """Roll the schema back to a revision.""" + from alembic import command + + cfg = _config() + try: + command.downgrade(cfg, revision) + except Exception as exc: + _err_console.print(f"[bold red]Downgrade failed:[/bold red] {exc}") + raise typer.Exit(1) from exc + _console.print(f"[bold green]OK[/bold green] schema downgraded to {revision!r}.") + + +@db_app.command(name="current") +def current() -> None: + """Show the current revision stamped in ``lfx_alembic_version``.""" + from alembic import command + + command.current(_config()) + + +@db_app.command(name="check") +def check() -> None: + """Fail (exit 1) if the models drift from the migrations. Use in CI.""" + from alembic import command + from alembic.util import exc as alembic_exc + + cfg = _config() + try: + command.check(cfg) + except alembic_exc.AutogenerateDiffsDetected as exc: + _err_console.print( + f"[bold red]Schema drift:[/bold red] models and migrations differ.\n{exc}\n" + "Generate a revision with 'lfx db revision -m \"...\" --autogenerate' " + "and land the equivalent change in langflow's stream." + ) + raise typer.Exit(1) from exc + except alembic_exc.CommandError as exc: + _err_console.print(f"[bold red]Check failed:[/bold red] {exc} (is the schema migrated? run 'lfx db upgrade')") + raise typer.Exit(1) from exc + _console.print("[bold green]OK[/bold green] models match migrations.") + + +@db_app.command(name="revision") +def revision( + message: str = typer.Option(..., "-m", "--message", help="Revision message."), + autogenerate: bool = typer.Option( # noqa: FBT001 + False, # noqa: FBT003 + "--autogenerate", + help="Diff models against the database to populate the revision.", + ), +) -> None: + """Create a new revision in lfx's migration stream.""" + from alembic import command + + cfg = _config() + try: + command.revision(cfg, message=message, autogenerate=autogenerate) + except Exception as exc: + _err_console.print(f"[bold red]Revision failed:[/bold red] {exc}") + raise typer.Exit(1) from exc + + +def register(app: typer.Typer) -> None: + """Register the ``db`` command group on the root lfx app.""" + app.add_typer(db_app, name="db", rich_help_panel="Database") diff --git a/src/lfx/src/lfx/cli/commands.py b/src/lfx/src/lfx/cli/commands.py index d088d15db305..dfc89c222119 100644 --- a/src/lfx/src/lfx/cli/commands.py +++ b/src/lfx/src/lfx/cli/commands.py @@ -216,6 +216,69 @@ async def _build_serve_registry( return registry, temp_file_to_cleanup +def _ensure_lfx_schema_migrated_or_exit() -> None: + """Explicit-migration boot policy for ``lfx serve``. + + lfx does not migrate implicitly on serve. When a real (persistent) Tier 1 + database is wired, verify its schema is at head and refuse to start + otherwise, pointing the operator at ``lfx db upgrade``. No-op for the bare + ephemeral path (NoopDatabaseService / use_noop_database), so ``lfx serve`` of + a stateless flow is unaffected. + + The check is synchronous and fork-safe: it never instantiates the async + engine, only the class (to read its migration-stream identity) plus a + short-lived sync connection. + """ + from lfx.services.database.service import ( + NoopDatabaseService, + check_schema_at_head_sync, + ) + from lfx.services.deps import get_settings_service + from lfx.services.manager import get_service_manager + from lfx.services.schema import ServiceType + + settings = getattr(get_settings_service(), "settings", None) + if settings is None or getattr(settings, "use_noop_database", False): + return + + manager = get_service_manager() + # wiring_manifest() triggers plugin discovery (loads lfx.toml) without + # instantiating anything. + manifest = manager.wiring_manifest() + entry = manifest.get(ServiceType.DATABASE_SERVICE) + if entry is None or entry.impl_class == NoopDatabaseService.__name__: + return # bare/ephemeral serving -- nothing to migrate + + db_cls = manager._resolve_class(ServiceType.DATABASE_SERVICE) # noqa: SLF001 + if db_cls is None or not hasattr(db_cls, "default_script_location"): + return + + database_url = getattr(settings, "database_url", None) + if not database_url: + return + + try: + at_head, current, head = check_schema_at_head_sync( + database_url, + script_location=db_cls.default_script_location(), + version_table=getattr(db_cls, "alembic_version_table", "lfx_alembic_version"), + ) + except Exception as exc: + # A check failure (DB unreachable, bad URL) must not be silently treated + # as "fine to serve" -- serving would fail anyway. Surface and stop. + typer.echo(f"Error: could not verify database schema before serving: {exc}", err=True) + raise typer.Exit(1) from exc + + if not at_head: + typer.echo( + f"Error: database schema is not at head (current={current!r}, head={head!r}). " + "lfx does not migrate automatically on serve. Run 'lfx db upgrade' against " + "LANGFLOW_DATABASE_URL (or an init container / job) before starting the server.", + err=True, + ) + raise typer.Exit(1) + + def serve_command( script_paths: list[str] | None = typer.Argument( default=None, @@ -399,6 +462,10 @@ def serve_command( else: source_display = "none (upload flows via POST /flows/upload/)" + # Explicit-migration boot policy: refuse to serve against an unmigrated + # persistent database (no-op for the bare ephemeral path). + _ensure_lfx_schema_migrated_or_exit() + temp_file_to_cleanup: str | None = None try: registry, temp_file_to_cleanup = asyncio.run( diff --git a/src/lfx/src/lfx/services/database/constants.py b/src/lfx/src/lfx/services/database/constants.py new file mode 100644 index 000000000000..af0348067917 --- /dev/null +++ b/src/lfx/src/lfx/services/database/constants.py @@ -0,0 +1,19 @@ +"""Database service constants (lfx). + +The minimum-PostgreSQL requirement is a property of the *shared* schema, whose +models live in ``lfx.services.database.models``: several tables use +``UNIQUE NULLS DISTINCT``, which only PostgreSQL 15+ supports. Because the schema +is owned here, the version floor is defined here too and langflow re-exports it. +""" + +# Minimum PostgreSQL major version required by the lfx/langflow schema. +MIN_POSTGRESQL_MAJOR_VERSION = 15 + +# User-facing message when migrations fail due to PostgreSQL < 15. +POSTGRESQL_VERSION_REQUIRED_MESSAGE = ( + f"PostgreSQL {MIN_POSTGRESQL_MAJOR_VERSION} or higher is required when using PostgreSQL as the database. " + "The current PostgreSQL version does not support the syntax used by the schema " + "(e.g. UNIQUE NULLS DISTINCT). " + f"Please upgrade your PostgreSQL instance to version {MIN_POSTGRESQL_MAJOR_VERSION} or higher. " + "See: https://docs.langflow.org/configuration-custom-database" +) diff --git a/src/lfx/src/lfx/services/database/migrations/README.md b/src/lfx/src/lfx/services/database/migrations/README.md new file mode 100644 index 000000000000..f47822942a33 --- /dev/null +++ b/src/lfx/src/lfx/services/database/migrations/README.md @@ -0,0 +1,42 @@ +# lfx migration stream + +This is lfx's **own** alembic lineage, independent of langflow's. + +- **Version table:** `lfx_alembic_version` (distinct from langflow's `alembic_version`). +- **Scope:** only the execution-history tables lfx owns — `message`, `transaction`, + `vertex_build` — enforced by the `include_name` / `include_object` filters in + [`env.py`](env.py) (`LFX_MIGRATION_TABLES`). + +## Why a separate stream + +A bare `lfx serve` production deployment must be able to provision its schema +without langflow's 82 migrations present. The model classes are the single +source of truth (they live in `lfx.services.database.models`), so both streams +materialise the *same* target — they can only differ in staleness, which +`alembic check` catches per-stream in CI. See the "two-stream migration model" +section of `src/lfx/PLUGGABLE_SERVICES.md`. + +A given physical database is owned by exactly **one** stream: the langflow +editor DB by langflow's lineage, the scaled `lfx serve` DB by this one. + +## Commands + +lfx does not migrate implicitly on `serve` (explicit-only policy). Provision with: + +```bash +lfx db upgrade # apply migrations to head +lfx db current # show the current revision +lfx db check # fail if models drift from the migrations (CI) +lfx db downgrade -1 # roll back one revision +``` + +## Authoring a new revision + +After changing an lfx-core model: + +```bash +lfx db revision -m "add message.foo" --autogenerate +``` + +Then **also** land the equivalent change in langflow's stream +(`src/backend/base/langflow/alembic`) — the shared model is visible to both. diff --git a/src/lfx/src/lfx/services/database/migrations/alembic.ini b/src/lfx/src/lfx/services/database/migrations/alembic.ini new file mode 100644 index 000000000000..59a1cd3ebf9c --- /dev/null +++ b/src/lfx/src/lfx/services/database/migrations/alembic.ini @@ -0,0 +1,49 @@ +# Alembic config for lfx's own migration stream. +# +# The lfx runtime (DatabaseService / `lfx db`) builds the alembic Config +# programmatically -- setting script_location and sqlalchemy.url from settings -- +# so this file is only used when driving migrations with the bare `alembic` CLI +# from this directory. `sqlalchemy.url` is intentionally left as a placeholder; +# pass `-x` / env or use `lfx db` instead. + +[alembic] +script_location = . +prepend_sys_path = . +version_path_separator = os +sqlalchemy.url = driver://user:pass@localhost/dbname + +[post_write_hooks] + +[loggers] +keys = root,sqlalchemy,alembic + +[handlers] +keys = console + +[formatters] +keys = generic + +[logger_root] +level = WARNING +handlers = console +qualname = + +[logger_sqlalchemy] +level = WARNING +handlers = +qualname = sqlalchemy.engine + +[logger_alembic] +level = INFO +handlers = +qualname = alembic + +[handler_console] +class = StreamHandler +args = (sys.stderr,) +level = NOTSET +formatter = generic + +[formatter_generic] +format = %(levelname)-5.5s [%(name)s] %(message)s +datefmt = %H:%M:%S diff --git a/src/lfx/src/lfx/services/database/migrations/env.py b/src/lfx/src/lfx/services/database/migrations/env.py new file mode 100644 index 000000000000..3686c27d66c0 --- /dev/null +++ b/src/lfx/src/lfx/services/database/migrations/env.py @@ -0,0 +1,209 @@ +"""Alembic environment for lfx's own migration stream. + +This is lfx's *independent* migration lineage. It stamps a distinct version +table (``lfx_alembic_version``) and, via the ``include_name`` / ``include_object`` +filters below, only ever considers the execution-history tables lfx owns +(``LFX_MIGRATION_TABLES``). That is the ownership boundary that keeps this stream +isolated from langflow's 82-migration lineage: + +* A langflow-only table (e.g. a future ``authz_*`` table, or a user's own model) + is invisible here -- the filters skip it -- so it never enters or perturbs + lfx's lineage. +* An lfx-core table change is visible to *both* streams' autogenerate, so it + needs a revision in each. ``alembic check`` (run per-stream in CI) fails the + build the moment a stream lags the shared model, so drift is loud, not silent. + +The ``NAMING_CONVENTION`` is intentionally byte-for-byte identical to langflow's +env so the same model yields identically-named indexes/constraints in both +streams -- itself a divergence-prevention measure. +""" + +import asyncio +import hashlib +import os +import warnings +from logging.config import fileConfig +from typing import Any + +from alembic import context +from lfx.log.logger import logger + +# Importing the model modules registers the lfx-core tables on SQLModel.metadata +# so autogenerate can see them. Only lfx-owned execution-history tables are +# imported here; the include filters below are the authoritative boundary. +from lfx.services.database.models import message as _message # noqa: F401 +from lfx.services.database.models import transactions as _transactions # noqa: F401 +from lfx.services.database.models import vertex_builds as _vertex_builds # noqa: F401 +from sqlalchemy import pool, text +from sqlalchemy.event import listen +from sqlalchemy.exc import SAWarning +from sqlalchemy.ext.asyncio import async_engine_from_config +from sqlmodel import SQLModel + +# The tables lfx's migration stream owns. Keep in sync with +# DatabaseService.expected_tables. ``lfx_alembic_version`` is included so alembic +# does not try to drop its own bookkeeping table. +LFX_MIGRATION_TABLES = { + "message", + "transaction", + "vertex_build", + "lfx_alembic_version", +} + +# The version table for lfx's stream -- distinct from langflow's default +# ``alembic_version`` so the two lineages never collide in one database. +LFX_VERSION_TABLE = "lfx_alembic_version" + +config = context.config + +if config.config_file_name is not None: + fileConfig(config.config_file_name) + +NAMING_CONVENTION = { + "ix": "ix_%(column_0_label)s", + "uq": "uq_%(table_name)s_%(column_0_name)s", + "ck": "ck_%(table_name)s_%(constraint_name)s", + "fk": "fk_%(table_name)s_%(column_0_name)s_%(referred_table_name)s", + "pk": "pk_%(table_name)s", +} + +target_metadata = SQLModel.metadata +target_metadata.naming_convention = NAMING_CONVENTION + + +def include_name(name, type_, parent_names) -> bool: # noqa: ARG001 + """Restrict reflection to lfx-owned tables. + + Without this, autogenerate reflecting a database that also holds langflow + tables would propose dropping them. Filtering by name keeps this stream blind + to everything but the tables it owns. + """ + if type_ == "table": + return name in LFX_MIGRATION_TABLES + return True + + +def include_object(obj, name, type_, reflected, compare_to) -> bool: # noqa: ARG001 + """Restrict metadata objects to lfx-owned tables (mirror of include_name).""" + if type_ == "table": + return name in LFX_MIGRATION_TABLES + return True + + +def _configure_kwargs(base: dict[str, Any]) -> dict[str, Any]: + base.update( + { + "target_metadata": target_metadata, + "version_table": LFX_VERSION_TABLE, + "include_name": include_name, + "include_object": include_object, + "render_as_batch": True, + } + ) + return base + + +def run_migrations_offline() -> None: + """Run migrations in 'offline' mode.""" + url = config.get_main_option("sqlalchemy.url") + configure_kwargs = _configure_kwargs( + { + "url": url, + "literal_binds": True, + "dialect_opts": {"paramstyle": "named"}, + } + ) + if url and "postgresql" in url: + configure_kwargs["prepare_threshold"] = None + + context.configure(**configure_kwargs) + + with context.begin_transaction(): + context.run_migrations() + + +def _sqlite_do_connect(dbapi_connection, connection_record): # noqa: ARG001 + # disable pysqlite's emitting of the BEGIN statement entirely. + dbapi_connection.isolation_level = None + + +def _sqlite_do_begin(conn): + # emit our own BEGIN + conn.exec_driver_sql("PRAGMA busy_timeout = 60000") + conn.exec_driver_sql("BEGIN EXCLUSIVE") + + +def _do_run_migrations(connection) -> None: + configure_kwargs = _configure_kwargs({"connection": connection}) + if connection.dialect.name == "postgresql": + configure_kwargs["prepare_threshold"] = None + + context.configure(**configure_kwargs) + with context.begin_transaction(): + if connection.dialect.name == "postgresql": + # Serialise concurrent migration runs across workers with an advisory + # lock so they do not race on CREATE TABLE against a fresh database. + namespace = os.getenv("LANGFLOW_MIGRATION_LOCK_NAMESPACE") + if namespace: + lock_key = int(hashlib.sha256(namespace.encode()).hexdigest()[:16], 16) % (2**63 - 1) + logger.info(f"Using migration lock namespace: {namespace}, lock_key: {lock_key}") + else: + lock_key = 11223344 + logger.info(f"Using default migration lock_key: {lock_key}") + + connection.execute(text("SET LOCAL lock_timeout = '180s';")) + connection.execute(text(f"SELECT pg_advisory_xact_lock({lock_key});")) + if connection.dialect.name == "sqlite": + with warnings.catch_warnings(): + warnings.filterwarnings( + "ignore", + message=".*SQL-parsed foreign key constraint.*could not be located in PRAGMA foreign_keys.*", + category=SAWarning, + ) + warnings.filterwarnings( + "ignore", + message=( + "autogenerate skipping metadata-specified expression-based index " + "'ix_message_session_metadata_(tenant|user)'; dialect 'sqlite'.*" + ), + category=UserWarning, + ) + context.run_migrations() + else: + context.run_migrations() + + +async def _run_async_migrations() -> None: + config_section = config.get_section(config.config_ini_section, {}) + db_url = config_section.get("sqlalchemy.url", "") + + connect_args: dict[str, Any] = {} + if db_url and "postgresql" in db_url: + connect_args["prepare_threshold"] = None + + connectable = async_engine_from_config( + config_section, + prefix="sqlalchemy.", + poolclass=pool.NullPool, + connect_args=connect_args, + ) + + if connectable.dialect.name == "sqlite": + listen(connectable.sync_engine, "connect", _sqlite_do_connect) + listen(connectable.sync_engine, "begin", _sqlite_do_begin) + + async with connectable.connect() as connection: + await connection.run_sync(_do_run_migrations) + + await connectable.dispose() + + +def run_migrations_online() -> None: + """Run migrations in 'online' mode.""" + asyncio.run(_run_async_migrations()) + + +if context.is_offline_mode(): + run_migrations_offline() +else: + run_migrations_online() diff --git a/src/lfx/src/lfx/services/database/migrations/script.py.mako b/src/lfx/src/lfx/services/database/migrations/script.py.mako new file mode 100644 index 000000000000..19bb7540f1fc --- /dev/null +++ b/src/lfx/src/lfx/services/database/migrations/script.py.mako @@ -0,0 +1,30 @@ +"""${message} + +Revision ID: ${up_revision} +Revises: ${down_revision | comma,n} +Create Date: ${create_date} + +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa +import sqlmodel +from sqlalchemy.engine.reflection import Inspector +${imports if imports else ""} + +# revision identifiers, used by Alembic. +revision: str = ${repr(up_revision)} +down_revision: Union[str, None] = ${repr(down_revision)} +branch_labels: Union[str, Sequence[str], None] = ${repr(branch_labels)} +depends_on: Union[str, Sequence[str], None] = ${repr(depends_on)} + + +def upgrade() -> None: + conn = op.get_bind() + ${upgrades if upgrades else "pass"} + + +def downgrade() -> None: + conn = op.get_bind() + ${downgrades if downgrades else "pass"} diff --git a/src/lfx/src/lfx/services/database/migrations/versions/35f161a1123e_initial_lfx_execution_history_schema.py b/src/lfx/src/lfx/services/database/migrations/versions/35f161a1123e_initial_lfx_execution_history_schema.py new file mode 100644 index 000000000000..52f337287adb --- /dev/null +++ b/src/lfx/src/lfx/services/database/migrations/versions/35f161a1123e_initial_lfx_execution_history_schema.py @@ -0,0 +1,91 @@ +"""initial lfx execution history schema + +Revision ID: 35f161a1123e +Revises: +Create Date: 2026-07-13 18:15:11.003221 + +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa +import sqlmodel +from sqlalchemy.engine.reflection import Inspector + + +# revision identifiers, used by Alembic. +revision: str = '35f161a1123e' +down_revision: Union[str, None] = None +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + conn = op.get_bind() + # ### commands auto generated by Alembic - please adjust! ### + op.create_table('message', + sa.Column('timestamp', sa.DateTime(), nullable=False), + sa.Column('sender', sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.Column('sender_name', sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.Column('session_id', sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.Column('context_id', sqlmodel.sql.sqltypes.AutoString(), nullable=True), + sa.Column('text', sa.Text(), nullable=True), + sa.Column('error', sa.Boolean(), nullable=False), + sa.Column('edit', sa.Boolean(), nullable=False), + sa.Column('files', sa.JSON(), nullable=True), + sa.Column('properties', sa.JSON(), nullable=True), + sa.Column('category', sa.Text(), nullable=True), + sa.Column('content_blocks', sa.JSON(), nullable=True), + sa.Column('session_metadata', sa.JSON(), nullable=True), + sa.Column('id', sa.Uuid(), nullable=False), + sa.Column('flow_id', sa.Uuid(), nullable=True), + sa.Column('run_id', sa.Uuid(), nullable=True), + sa.Column('is_output', sa.Boolean(), nullable=False), + sa.PrimaryKeyConstraint('id') + ) + with op.batch_alter_table('message', schema=None) as batch_op: + batch_op.create_index(batch_op.f('ix_message_run_id'), ['run_id'], unique=False) + + op.create_table('transaction', + sa.Column('timestamp', sa.DateTime(), nullable=False), + sa.Column('vertex_id', sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.Column('target_id', sqlmodel.sql.sqltypes.AutoString(), nullable=True), + sa.Column('inputs', sa.JSON(), nullable=True), + sa.Column('outputs', sa.JSON(), nullable=True), + sa.Column('status', sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.Column('error', sqlmodel.sql.sqltypes.AutoString(), nullable=True), + sa.Column('flow_id', sa.Uuid(), nullable=False), + sa.Column('id', sa.Uuid(), nullable=False), + sa.PrimaryKeyConstraint('id') + ) + op.create_table('vertex_build', + sa.Column('timestamp', sa.DateTime(), nullable=False), + sa.Column('id', sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.Column('data', sa.JSON(), nullable=True), + sa.Column('artifacts', sa.JSON(), nullable=True), + sa.Column('params', sa.Text(), nullable=True), + sa.Column('valid', sa.Boolean(), nullable=False), + sa.Column('flow_id', sa.Uuid(), nullable=False), + sa.Column('job_id', sa.Uuid(), nullable=True), + sa.Column('build_id', sa.Uuid(), nullable=False), + sa.PrimaryKeyConstraint('build_id') + ) + with op.batch_alter_table('vertex_build', schema=None) as batch_op: + batch_op.create_index(batch_op.f('ix_vertex_build_job_id'), ['job_id'], unique=False) + + # ### end Alembic commands ### + + +def downgrade() -> None: + conn = op.get_bind() + # ### commands auto generated by Alembic - please adjust! ### + with op.batch_alter_table('vertex_build', schema=None) as batch_op: + batch_op.drop_index(batch_op.f('ix_vertex_build_job_id')) + + op.drop_table('vertex_build') + op.drop_table('transaction') + with op.batch_alter_table('message', schema=None) as batch_op: + batch_op.drop_index(batch_op.f('ix_message_run_id')) + + op.drop_table('message') + # ### end Alembic commands ### diff --git a/src/lfx/src/lfx/services/database/service.py b/src/lfx/src/lfx/services/database/service.py index f2ff4a4ccd4b..ee7cd8adc056 100644 --- a/src/lfx/src/lfx/services/database/service.py +++ b/src/lfx/src/lfx/services/database/service.py @@ -1,17 +1,63 @@ -"""Database service implementations for lfx package.""" +"""Database service implementations for lfx package. + +Two implementations live here: + +* :class:`NoopDatabaseService` -- the zero-dependency default for bare ``lfx`` + usage; every session is a :class:`~lfx.services.session.NoopSession`. +* :class:`DatabaseService` -- the real Tier 1 infrastructure service: a pooled + async SQLAlchemy engine plus an alembic migration runner over lfx's own + migration stream (``lfx.services.database.migrations``, version table + ``lfx_alembic_version``). + +langflow subclasses :class:`DatabaseService` (thin override) to point the +migration runner at its own alembic tree and to add its domain bootstrap +(superuser assignment, schema-health checks). The engine/session/migration +*mechanism* is shared; only the migration-stream identity and domain policy +differ. See ``PLUGGABLE_SERVICES.md`` (two-stream migration model) for why the +two alembic lineages stay isolated. +""" from __future__ import annotations -from contextlib import asynccontextmanager +import asyncio +import os +import sqlite3 +import sys +import time +from contextlib import asynccontextmanager, contextmanager, nullcontext +from datetime import datetime, timezone +from pathlib import Path from typing import TYPE_CHECKING, ClassVar +import sqlalchemy as sa +from alembic import command, util +from alembic.config import Config +from sqlalchemy import event +from sqlalchemy.dialects import sqlite as dialect_sqlite +from sqlalchemy.engine import Engine, make_url +from sqlalchemy.exc import OperationalError +from sqlalchemy.ext.asyncio import AsyncEngine, async_sessionmaker, create_async_engine +from sqlmodel import SQLModel, text +from sqlmodel.ext.asyncio.session import AsyncSession as SQLModelAsyncSession +from tenacity import retry, stop_after_attempt, wait_fixed + +from lfx.log.logger import logger +from lfx.services.base import Service from lfx.services.capabilities import Capability, Tier +from lfx.services.database.constants import ( + MIN_POSTGRESQL_MAJOR_VERSION, + POSTGRESQL_VERSION_REQUIRED_MESSAGE, +) +from lfx.services.deps import session_scope +from lfx.utils.windows_postgres_helper import configure_windows_postgres_event_loop if TYPE_CHECKING: from collections.abc import AsyncGenerator from sqlalchemy.ext.asyncio import AsyncSession + from lfx.services.settings.service import SettingsService + class NoopDatabaseService: """No-operation database service for standalone lfx usage. @@ -23,7 +69,7 @@ class NoopDatabaseService: ``NoopSession`` neither persists across restarts nor shares state across processes. A Tier 2 service that ``Requires`` the database with ``{PERSISTENT}`` therefore fails ``validate_wiring()`` against this - implementation — which is the desired loud-at-boot behavior instead of + implementation -- which is the desired loud-at-boot behavior instead of silent no-op writes. """ @@ -60,3 +106,790 @@ def session_scope_readonly(self) -> AsyncGenerator[AsyncSession, None]: from lfx.services.database.session import session_scope_readonly_for return session_scope_readonly_for(self) + + +# --------------------------------------------------------------------------- +# Migration + connection helpers (module-level, dialect-aware, pure). +# --------------------------------------------------------------------------- + + +class UnsupportedPostgreSQLVersionError(Exception): + """Raised when the PostgreSQL version is below the minimum required.""" + + +_PG_VERSION_QUERY = sa.text("SELECT current_setting('server_version_num'), current_setting('server_version')") + +# Stable namespace for the schema-migration advisory lock. The lock serializes +# concurrent ``alembic upgrade`` runs across workers so they do not race to +# CREATE TYPE / CREATE TABLE on a fresh database. Picked once and never changed +# so independent processes converge on the same lock; the value itself is +# arbitrary, just has to fit in a Postgres bigint and not collide with other +# advisory locks the application takes (currently none). +_MIGRATION_ADVISORY_LOCK_ID = 0x4C616E67666C6F77 # ASCII "Langflow" +_MIGRATION_LOCK_DEFAULT_TIMEOUT_S = 300.0 +_MIGRATION_LOCK_POLL_INTERVAL_S = 2.0 + + +def _migration_lock_timeout_s() -> float: + raw = os.getenv("LANGFLOW_MIGRATION_LOCK_TIMEOUT_S") + if raw is None: + return _MIGRATION_LOCK_DEFAULT_TIMEOUT_S + try: + return float(raw) + except ValueError: + logger.warning( + "Ignoring invalid LANGFLOW_MIGRATION_LOCK_TIMEOUT_S=%r; falling back to %.0fs.", + raw, + _MIGRATION_LOCK_DEFAULT_TIMEOUT_S, + ) + return _MIGRATION_LOCK_DEFAULT_TIMEOUT_S + + +def _acquire_migration_lock_or_raise(conn, lock_id: int) -> None: + """Acquire the advisory lock with a bounded wait, logging progress. + + Blocking ``pg_advisory_lock`` has no upper bound and ``lock_timeout`` does + not apply to advisory locks, so a worker hung mid-migration would silently + block every other worker forever. Instead poll ``pg_try_advisory_lock`` with + a configurable timeout and log when we're waiting, so operators see why + boot is stuck. + """ + if conn.execute(sa.text(f"SELECT pg_try_advisory_lock({lock_id})")).scalar(): + return + + timeout = _migration_lock_timeout_s() + logger.info( + "Migration advisory lock %s held by another worker; waiting up to %.0fs.", + lock_id, + timeout, + ) + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + time.sleep(_MIGRATION_LOCK_POLL_INTERVAL_S) + if conn.execute(sa.text(f"SELECT pg_try_advisory_lock({lock_id})")).scalar(): + logger.info("Acquired migration advisory lock %s after waiting.", lock_id) + return + + msg = ( + f"Could not acquire migration advisory lock {lock_id} within " + f"{timeout:.0f}s. Another worker is likely hung mid-migration. " + "Investigate the worker holding the lock or restart the deployment " + "with a single worker so migrations can run cleanly. Override the " + "wait via LANGFLOW_MIGRATION_LOCK_TIMEOUT_S (seconds) if your migration " + "legitimately needs longer." + ) + raise RuntimeError(msg) + + +def _normalize_sync_postgres_url(database_url: str) -> str: + """Return a sync-driver Postgres URL from a possibly async one. + + Strips the ``+asyncpg`` / ``+aiosqlite`` suffix and upgrades the legacy + ``postgres://`` scheme to ``postgresql://`` so :func:`sa.create_engine` + picks the default sync driver. Centralised so the advisory-lock helper and + the table-creation lock path stay in sync with :func:`check_postgresql_version_sync`. + """ + sync_url = database_url + if sync_url.startswith("postgres://"): + sync_url = "postgresql://" + sync_url.split("://", 1)[1] + for async_driver in ("+asyncpg", "+aiosqlite"): + sync_url = sync_url.replace(async_driver, "") + return sync_url + + +@contextmanager +def _postgres_migration_lock(database_url: str): + """Hold a Postgres session-level advisory lock for the duration of the block. + + Workers starting concurrently against a fresh PostgreSQL each call + ``command.upgrade("head")``; without coordination they race on + ``CREATE TYPE`` / ``CREATE TABLE`` and the losers fail with + ``UniqueViolation``. Holding a session-level advisory lock serialises the + upgrade so only one worker mutates the schema at a time; the others wait + here (bounded, with progress logging) and then find the schema already at + head. + + No-op for non-PostgreSQL URLs. SQLite has no advisory locks (and runs + single-process anyway). + """ + if not database_url.startswith(("postgresql", "postgres")): + yield + return + + engine = sa.create_engine(_normalize_sync_postgres_url(database_url)) + try: + with engine.connect() as conn: + logger.debug("Acquiring migration advisory lock %s", _MIGRATION_ADVISORY_LOCK_ID) + _acquire_migration_lock_or_raise(conn, _MIGRATION_ADVISORY_LOCK_ID) + try: + yield + finally: + logger.debug("Releasing migration advisory lock %s", _MIGRATION_ADVISORY_LOCK_ID) + # Session-level locks auto-release on connection close, but + # explicit unlock keeps the connection reusable if alembic + # internals ever hand us one back. + conn.execute(sa.text(f"SELECT pg_advisory_unlock({_MIGRATION_ADVISORY_LOCK_ID})")) + finally: + engine.dispose() + + +def _check_version_row(version_num_str: str, version_str: str) -> None: + """Raise ``UnsupportedPostgreSQLVersionError`` when the version is too old.""" + if int(version_num_str) < MIN_POSTGRESQL_MAJOR_VERSION * 10000: + msg = f"Running PostgreSQL {version_str}. {POSTGRESQL_VERSION_REQUIRED_MESSAGE}" + logger.error(msg) + raise UnsupportedPostgreSQLVersionError(msg) + + +def check_postgresql_version_sync(database_url: str) -> None: + """Pre-flight check: verify PostgreSQL >= 15 using a synchronous connection. + + Call this *before* starting uvicorn/gunicorn so a version mismatch + results in a clean ``sys.exit(1)`` rather than a messy lifespan failure. + Silently returns when the URL is not PostgreSQL. + """ + if not database_url.startswith(("postgresql", "postgres")): + return + + from sqlalchemy import create_engine + + engine = create_engine(_normalize_sync_postgres_url(database_url)) + try: + with engine.connect() as conn: + row = conn.execute(_PG_VERSION_QUERY).fetchone() + _check_version_row(*row) + finally: + engine.dispose() + + +def get_sqlite_database_file_path(database_url: str) -> Path | None: + """Return the on-disk file path for a SQLite URL, or ``None`` when there is none. + + Returns ``None`` for non-SQLite URLs and for in-memory SQLite databases + (``sqlite://`` and ``sqlite:///:memory:``), which have no file on disk. The + returned path is kept exactly as written in the URL (relative paths are *not* + resolved) so callers can report it back to the user verbatim. + """ + if not database_url.startswith("sqlite"): + return None + try: + database = make_url(database_url).database + except Exception: # noqa: BLE001 - defensive: malformed URLs are handled elsewhere + return None + if not database or database == ":memory:": + return None + return Path(database) + + +def to_async_driver_url(database_url: str) -> str: + """Rewrite a bare dialect URL to its async driver form. + + ``sqlite`` -> ``sqlite+aiosqlite``; ``postgresql`` / ``postgres`` -> + ``postgresql+psycopg``. Already-qualified drivers pass through unchanged. + Shared by :meth:`DatabaseService._sanitize_database_url` and the ``lfx db`` + CLI so both drive the same async driver. + """ + driver, _, rest = database_url.partition("://") + if not rest: + return database_url + if driver == "sqlite": + driver = "sqlite+aiosqlite" + elif driver in {"postgresql", "postgres"}: + driver = "postgresql+psycopg" + return f"{driver}://{rest}" + + +def check_sqlite_database_path(database_url: str) -> None: + """Fail fast with an actionable message when a SQLite database cannot be opened. + + SQLite does not create intermediate directories, and relative paths in + ``LANGFLOW_DATABASE_URL`` are resolved by SQLAlchemy against the current + working directory at connect time. When the resolved parent directory is + missing the raw ``sqlite3.OperationalError`` ("unable to open database file") + is opaque, so surface where the database open was actually attempted and how + a relative path was resolved. No-op for non-SQLite and in-memory URLs. + + Note: this only improves diagnostics; it does not change which URLs are + accepted nor create any directories. + """ + db_path = get_sqlite_database_file_path(database_url) + if db_path is None: + return + + resolved = db_path.resolve() + logger.debug(f"Using SQLite database at {resolved}") + + parent = resolved.parent + if parent.exists(): + return + + msg = ( + f"Cannot open the SQLite database at '{resolved}': the parent directory " + f"'{parent}' does not exist, and SQLite does not create intermediate " + f"directories. " + ) + if db_path.is_absolute(): + msg += "Create the directory before starting the server, or point LANGFLOW_DATABASE_URL at an existing path." + else: + msg += ( + f"The relative path '{db_path}' from LANGFLOW_DATABASE_URL was resolved against the current working " + f"directory ('{Path.cwd()}'). Set LANGFLOW_DATABASE_URL to an absolute path " + f"(e.g. 'sqlite:///{resolved}'), or create the directory before starting the server." + ) + raise ValueError(msg) + + +def check_schema_at_head_sync( + database_url: str, + *, + script_location: Path, + version_table: str, +) -> tuple[bool, str | None, str | None]: + """Synchronously check whether ``database_url``'s schema is at migration head. + + Fork-safe pre-flight for ``lfx serve``: opens a short-lived sync connection, + reads the current revision from ``version_table``, and compares it to the + stream's head. Returns ``(at_head, current, head)``. A missing version table + reads as ``current=None`` (never migrated). Never creates the async engine, + so it is safe to call in the parent before workers fork. + """ + from alembic.runtime.migration import MigrationContext + from alembic.script import ScriptDirectory + + script = ScriptDirectory(str(script_location)) + head = script.get_current_head() + + engine = sa.create_engine(_normalize_sync_postgres_url(database_url)) + try: + with engine.connect() as conn: + ctx = MigrationContext.configure(conn, opts={"version_table": version_table}) + current = ctx.get_current_revision() + finally: + engine.dispose() + + return current == head, current, head + + +class SchemaNotAtHeadError(RuntimeError): + """Raised when a boot-time schema check finds pending migrations. + + lfx does not migrate implicitly on ``serve`` (explicit-only policy): the + operator runs ``lfx db upgrade`` (or an init container) first. This error + tells them exactly that. + """ + + +class DatabaseService(Service): + """Tier 1 infrastructure service: pooled async engine + alembic runner. + + The migration *stream* (which alembic tree, which version table) is + overridable so langflow's subclass can drive its own lineage while sharing + every line of engine/session/migration mechanism. Defaults target lfx's own + stream so a bare ``lfx`` install is self-sufficient. + """ + + name = "database_service" + + # Tier 1 infrastructure port. A real engine persists across restarts; SHARED + # (cross-process) is only true for Postgres, so the static class-level + # guarantee is the intersection: {PERSISTENT}. (A backend that wants to + # advertise SHARED can override per deployment.) + tier: ClassVar[Tier] = Tier.INFRASTRUCTURE + capabilities: ClassVar[frozenset[Capability]] = frozenset({Capability.PERSISTENT}) + + # --- Migration-stream identity (overridable by subclasses) ------------- + # The alembic version table this service's stream stamps. lfx uses a + # distinct name from langflow's default ``alembic_version`` so the two + # lineages never collide if they ever meet in the same database. + alembic_version_table: ClassVar[str] = "lfx_alembic_version" + # Tables this service is responsible for provisioning, used only for the + # post-``create_db_and_tables`` sanity check. langflow overrides with its + # fuller set. Migrations remain the source of truth in production. + expected_tables: ClassVar[tuple[str, ...]] = ("message", "transaction", "vertex_build") + + def session_scope(self): + """Async write session scope over this service (auto-commit/rollback). + + Part of the Tier 1 database port (Option B): Tier 2 services call this on + their injected ``database_service``. Delegates to the shared helper so + semantics match the module-level ``lfx.services.deps.session_scope``. + """ + from lfx.services.database.session import session_scope_for + + return session_scope_for(self) + + def session_scope_readonly(self): + """Read-only session scope over this service (no commit/rollback).""" + from lfx.services.database.session import session_scope_readonly_for + + return session_scope_readonly_for(self) + + def __init__(self, settings_service: SettingsService): + self._logged_pragma = False + self.settings_service = settings_service + if settings_service.settings.database_url is None: + msg = "No database URL provided" + raise ValueError(msg) + self.database_url: str = settings_service.settings.database_url + + configure_windows_postgres_event_loop(source="database_service") + + self._sanitize_database_url() + + # Migration-stream location: subclasses override to point at their own + # alembic tree. Defaults to lfx's own migrations package. + self.script_location = self._resolve_script_location() + self.alembic_cfg_path = self.script_location / "alembic.ini" + + # register the event listener for sqlite as part of this class. + # Using decorator will make the method not able to use self + event.listen(Engine, "connect", self.on_connection) + if self.settings_service.settings.database_connection_retry: + self.engine = self._create_engine_with_retry() + else: + self.engine = self._create_engine() + + # Create async session maker for efficient session creation + # This is the recommended SQLAlchemy 2.0+ pattern + # IMPORTANT: Must use SQLModel's AsyncSession (not SQLAlchemy's) for exec() method + self.async_session_maker = async_sessionmaker( + self.engine, + class_=SQLModelAsyncSession, # SQLModel's AsyncSession with exec() support + expire_on_commit=False, + ) + + # Check if Alembic should log to stdout or a file. + # If file, check if the provided path is absolute, cross-platform. + alembic_log_file = self.settings_service.settings.alembic_log_file + self.alembic_log_to_stdout = self.settings_service.settings.alembic_log_to_stdout + if self.alembic_log_to_stdout: + self.alembic_log_path = None + elif Path(alembic_log_file).is_absolute(): + self.alembic_log_path = Path(alembic_log_file) + else: + # Resolve relative log paths against the writable runtime config + # directory, not the installed package directory. The package dir is + # read-only in hardened deployments (non-root containers, read-only + # root filesystems, Kubernetes), where writing into it raises OSError + # and crashes startup. config_dir is always writable (it defaults to + # platformdirs' user cache dir and is created on startup). + config_dir = getattr(self.settings_service.settings, "config_dir", None) + base_dir = Path(config_dir) if config_dir else self.script_location.parent + self.alembic_log_path = base_dir / alembic_log_file + + @classmethod + def default_script_location(cls) -> Path: + """Alembic ``script_location`` for lfx's stream, resolvable without an instance. + + Ships inside the package so a wheel-only install can run ``lfx db upgrade``. + Exposed as a classmethod so the CLI can build a Config without constructing + the service (and thus without opening an engine). + """ + return Path(__file__).parent / "migrations" + + def _resolve_script_location(self) -> Path: + """Return the alembic ``script_location`` for this service's stream. + + Subclasses override to drive a different lineage (langflow points this at + its own alembic tree). + """ + return self.default_script_location() + + @classmethod + def make_cli_config(cls, settings_service: SettingsService, *, stdout=None) -> Config: + """Build an alembic ``Config`` for this stream without opening an engine. + + The ``lfx db`` commands drive alembic directly (``command.upgrade`` etc.); + the env module creates its own short-lived engine, so we deliberately do + not instantiate the service here. + """ + url = settings_service.settings.database_url + if url is None: + msg = "No database URL provided" + raise ValueError(msg) + cfg = Config(stdout=stdout if stdout is not None else sys.stdout) + cfg.set_main_option("script_location", str(cls.default_script_location())) + cfg.set_main_option("sqlalchemy.url", to_async_driver_url(url).replace("%", "%%")) + return cfg + + async def initialize_alembic_log_file(self): + log_path = self.alembic_log_path + if self.alembic_log_to_stdout or log_path is None: + return + + # Ensure the directory and file for the alembic log file exists. The + # migration log is diagnostic-only, so a read-only filesystem (hardened + # containers / Kubernetes) must never abort startup: warn and move on. + def _touch() -> None: + log_path.parent.mkdir(parents=True, exist_ok=True) + log_path.touch(exist_ok=True) + + try: + await asyncio.to_thread(_touch) + except OSError as exc: + await logger.awarning( + f"Could not initialize the Alembic migration log at '{log_path}' ({exc}). " + "Migration output falls back to stdout. Set LANGFLOW_ALEMBIC_LOG_FILE to a writable path " + "or LANGFLOW_ALEMBIC_LOG_TO_STDOUT=true to silence this warning." + ) + + def reload_engine(self) -> None: + self._sanitize_database_url() + if self.settings_service.settings.database_connection_retry: + self.engine = self._create_engine_with_retry() + else: + self.engine = self._create_engine() + + self.async_session_maker = async_sessionmaker( + self.engine, + class_=SQLModelAsyncSession, + expire_on_commit=False, + ) + + def _sanitize_database_url(self): + """Normalise the database URL to an async driver in place.""" + driver = self.database_url.split("://", maxsplit=1)[0] + if driver == "postgres": + logger.warning( + "The postgres dialect in the database URL is deprecated. " + "Use postgresql instead. " + "To avoid this warning, update the database URL." + ) + self.database_url = to_async_driver_url(self.database_url) + + def _build_connection_kwargs(self): + """Build connection kwargs by merging deprecated settings with db_connection_settings. + + Returns: + dict: Connection kwargs with deprecated settings overriding db_connection_settings + """ + settings = self.settings_service.settings + # Start with db_connection_settings as base + connection_kwargs = settings.db_connection_settings.copy() + + # Override individual settings if explicitly set + if "pool_size" in settings.model_fields_set: + logger.warning("pool_size is deprecated. Use db_connection_settings['pool_size'] instead.") + connection_kwargs["pool_size"] = settings.pool_size + if "max_overflow" in settings.model_fields_set: + logger.warning("max_overflow is deprecated. Use db_connection_settings['max_overflow'] instead.") + connection_kwargs["max_overflow"] = settings.max_overflow + + return connection_kwargs + + def _create_engine(self) -> AsyncEngine: + # Get connection settings from config, with defaults if not specified + # if the user specifies an empty dict, we allow it. + kwargs = self._build_connection_kwargs() + + poolclass_key = kwargs.get("poolclass") + if poolclass_key is not None: + pool_class = getattr(sa.pool, poolclass_key, None) + if pool_class and issubclass(pool_class, sa.pool.Pool): + logger.debug(f"Using poolclass: {poolclass_key}.") + kwargs["poolclass"] = pool_class + else: + logger.error(f"Invalid poolclass '{poolclass_key}' specified. Using default pool class.") + kwargs.pop("poolclass", None) + + return create_async_engine( + self.database_url, + connect_args=self._get_connect_args(), + **kwargs, + ) + + @retry(wait=wait_fixed(2), stop=stop_after_attempt(10)) + def _create_engine_with_retry(self) -> AsyncEngine: + """Create the engine for the database with retry logic.""" + return self._create_engine() + + def _get_connect_args(self): + settings = self.settings_service.settings + + if settings.db_driver_connection_settings is not None: + return settings.db_driver_connection_settings + + if settings.database_url and settings.database_url.startswith("sqlite"): + return { + "check_same_thread": False, + "timeout": settings.db_connect_timeout, + } + # For PostgreSQL, set the timezone to UTC + if settings.database_url and settings.database_url.startswith(("postgresql", "postgres")): + return {"options": "-c timezone=utc"} + return {} + + def on_connection(self, dbapi_connection, _connection_record) -> None: + if isinstance(dbapi_connection, sqlite3.Connection | dialect_sqlite.aiosqlite.AsyncAdapt_aiosqlite_connection): + pragmas: dict = self.settings_service.settings.sqlite_pragmas or {} + pragmas_list = [] + for key, val in pragmas.items(): + pragmas_list.append(f"PRAGMA {key} = {val}") + if not self._logged_pragma: + logger.debug(f"sqlite connection, setting pragmas: {pragmas_list}") + self._logged_pragma = True + if pragmas_list: + cursor = dbapi_connection.cursor() + try: + for pragma in pragmas_list: + try: + cursor.execute(pragma) + except OperationalError: + logger.exception(f"Failed to set PRAGMA {pragma}") + except GeneratorExit: + logger.error(f"Failed to set PRAGMA {pragma}") + finally: + cursor.close() + + @asynccontextmanager + async def _with_session(self): + """Internal method to create a session. DO NOT USE DIRECTLY. + + Use session_scope() for write operations or session_scope_readonly() for read operations. + This method does not handle commits - it only provides a raw session. + """ + if self.settings_service.settings.use_noop_database: + from lfx.services.session import NoopSession + + async with NoopSession() as session: + yield session + else: + # Use async_session_maker - the recommended SQLAlchemy 2.0+ pattern + # Provides efficient session creation and proper connection pooling + async with self.async_session_maker() as session: + yield session + + async def ensure_postgresql_version(self) -> None: + """If the database is PostgreSQL, ensure it is version 15 or higher. + + The schema uses UNIQUE NULLS DISTINCT, which is only supported in PostgreSQL 15+. + Logs the message and raises UnsupportedPostgreSQLVersionError if the version is too old. + """ + if not self.database_url.startswith(("postgresql", "postgres")): + return + if self.settings_service.settings.use_noop_database: + return + async with session_scope() as session: + result = await session.execute(_PG_VERSION_QUERY) + version_num_str, version_str = result.fetchone() + # Raise AFTER session_scope exits so session_scope doesn't log a + # noisy "An error occurred during the session scope." traceback. + _check_version_row(version_num_str, version_str) + + # --- Alembic migration runner ------------------------------------------ + + def _make_alembic_config(self, buffer) -> Config: + alembic_cfg = Config(stdout=buffer) + alembic_cfg.set_main_option("script_location", str(self.script_location)) + alembic_cfg.set_main_option("sqlalchemy.url", self.database_url.replace("%", "%%")) + return alembic_cfg + + def init_alembic(self, alembic_cfg) -> None: + logger.info("Initializing alembic") + command.ensure_version(alembic_cfg) + command.upgrade(alembic_cfg, "head") + + def _open_alembic_log_buffer(self): + """Open the Alembic migration log for writing, falling back to stdout. + + The migration log is diagnostic-only output. If the target path cannot + be written -- e.g. the installed package directory or the root + filesystem is read-only, as in hardened container/Kubernetes deployments + (non-root user or read-only root filesystem) -- migration must not abort. + Fall back to stdout rather than letting OSError propagate. Returns a + context manager yielding the buffer Alembic writes its output to. + """ + log_path = self.alembic_log_path + if self.alembic_log_to_stdout or log_path is None: + return nullcontext(sys.stdout) + try: + log_path.parent.mkdir(parents=True, exist_ok=True) + return log_path.open("w", encoding="utf-8") + except OSError as exc: + logger.warning( + f"Could not open the Alembic migration log at '{log_path}' ({exc}). " + "Falling back to stdout. Set LANGFLOW_ALEMBIC_LOG_FILE to a writable path " + "or LANGFLOW_ALEMBIC_LOG_TO_STDOUT=true to silence this warning." + ) + return nullcontext(sys.stdout) + + def _run_migrations(self, should_initialize_alembic, fix) -> None: + # The advisory lock serialises concurrent migration runs across workers + # so they do not race on CREATE TYPE / CREATE TABLE against a fresh PG. + buffer_context = self._open_alembic_log_buffer() + with _postgres_migration_lock(self.database_url), buffer_context as buffer: + alembic_cfg = self._make_alembic_config(buffer) + + if should_initialize_alembic: + try: + self.init_alembic(alembic_cfg) + except Exception as exc: + msg = f"Error initializing alembic: {exc}" + logger.exception(msg) + raise RuntimeError(msg) from exc + else: + logger.debug("Alembic initialized") + + try: + buffer.write(f"{datetime.now(tz=timezone.utc).astimezone().isoformat()}: Checking migrations\n") + command.check(alembic_cfg) + except Exception as exc: # noqa: BLE001 + logger.debug(f"Error checking migrations: {exc}") + if isinstance(exc, util.exc.CommandError | util.exc.AutogenerateDiffsDetected): + command.upgrade(alembic_cfg, "head") + time.sleep(3) + + try: + buffer.write(f"{datetime.now(tz=timezone.utc).astimezone()}: Checking migrations\n") + command.check(alembic_cfg) + except util.exc.AutogenerateDiffsDetected as exc: + logger.exception("Error checking migrations") + if not fix: + msg = f"There's a mismatch between the models and the database.\n{exc}" + raise RuntimeError(msg) from exc + + if fix: + self.try_downgrade_upgrade_until_success(alembic_cfg) + + async def run_migrations(self, *, fix=False) -> None: + should_initialize_alembic = False + async with session_scope() as session: + # If the version table does not exist it throws an error, so we + # catch it and initialize this stream's alembic bookkeeping. + try: + await session.exec(text(f"SELECT * FROM {self.alembic_version_table}")) # noqa: S608 + except Exception: # noqa: BLE001 + await logger.adebug("Alembic not initialized") + should_initialize_alembic = True + await asyncio.to_thread(self._run_migrations, should_initialize_alembic, fix) + + @staticmethod + def try_downgrade_upgrade_until_success(alembic_cfg, retries=5) -> None: + # Try -1 then head, if it fails, try -2 then head, etc. + # until we reach the number of retries + for i in range(1, retries + 1): + try: + command.check(alembic_cfg) + break + except util.exc.AutogenerateDiffsDetected: + # downgrade to base and upgrade again + logger.warning("AutogenerateDiffsDetected") + command.downgrade(alembic_cfg, f"-{i}") + # wait for the database to be ready + time.sleep(3) + command.upgrade(alembic_cfg, "head") + + async def schema_is_at_head(self) -> bool: + """Return True when this stream's schema is fully migrated. + + Used by the explicit-migration boot policy: ``lfx serve`` verifies the + schema is at head and refuses to start otherwise (rather than migrating + implicitly). Returns False when the version table is missing or when + alembic reports pending autogenerate diffs. + """ + # First, cheap existence check: no version table means "never migrated". + async with session_scope() as session: + try: + await session.exec(text(f"SELECT * FROM {self.alembic_version_table}")) # noqa: S608 + except Exception: # noqa: BLE001 + return False + return await asyncio.to_thread(self._check_at_head) + + def _check_at_head(self) -> bool: + with nullcontext(sys.stdout) as buffer: + alembic_cfg = self._make_alembic_config(buffer) + try: + command.check(alembic_cfg) + except util.exc.AutogenerateDiffsDetected: + return False + except util.exc.CommandError: + # e.g. target database is not up to date + return False + return True + + async def ensure_schema_at_head_or_raise(self) -> None: + """Raise :class:`SchemaNotAtHeadError` unless the schema is at head. + + The explicit-only migration policy: bare ``lfx serve`` does not migrate + implicitly. Operators run ``lfx db upgrade`` (or an init container). + """ + if self.settings_service.settings.use_noop_database: + return + if await self.schema_is_at_head(): + return + msg = ( + "Database schema is not at head (pending migrations). lfx does not migrate " + "automatically on serve. Run 'lfx db upgrade' (or an init container / job) " + "against LANGFLOW_DATABASE_URL before starting the server." + ) + raise SchemaNotAtHeadError(msg) + + # --- create_all fallback (dev/test convenience; migrations own prod) ---- + + def _create_db_and_tables(self, connection) -> None: + from sqlalchemy import inspect + + inspector = inspect(connection) + table_names = inspector.get_table_names() + current_tables = list(self.expected_tables) + + if table_names and all(table in table_names for table in current_tables): + logger.debug("Database and tables already exist") + return + + logger.debug("Creating database and tables") + + for table in SQLModel.metadata.sorted_tables: + try: + table.create(connection, checkfirst=True) + except OperationalError as oe: + logger.warning(f"Table {table} already exists, skipping. Exception: {oe}") + except Exception as exc: + msg = f"Error creating table {table}" + logger.exception(msg) + raise RuntimeError(msg) from exc + + # Now check if the required tables exist, if not, something went wrong. + inspector = inspect(connection) + table_names = inspector.get_table_names() + for table in current_tables: + if table not in table_names: + logger.error("Something went wrong creating the database and tables.") + logger.error("Please check your database settings.") + msg = "Something went wrong creating the database and tables." + raise RuntimeError(msg) + + logger.debug("Database and tables created successfully") + + @retry(wait=wait_fixed(2), stop=stop_after_attempt(10)) + async def create_db_and_tables_with_retry(self) -> None: + await self.create_db_and_tables() + + async def create_db_and_tables(self) -> None: + if not self.database_url.startswith(("postgresql", "postgres")): + # SQLite / non-PG: original async path; advisory lock does not apply. + async with self.engine.begin() as conn: + await conn.run_sync(self._create_db_and_tables) + return + + # Postgres: serialise CREATE TYPE / CREATE TABLE across workers under + # the same advisory lock that protects run_migrations. + await asyncio.to_thread(self._create_db_and_tables_with_lock) + + def _create_db_and_tables_with_lock(self) -> None: + """Postgres path: hold the migration advisory lock for the DDL. + + Opens its own sync engine so the DDL runs on the same driver the lock + uses; the application's async engine is unaffected. + """ + with _postgres_migration_lock(self.database_url): + sync_engine = sa.create_engine(_normalize_sync_postgres_url(self.database_url)) + try: + with sync_engine.begin() as conn: + self._create_db_and_tables(conn) + finally: + sync_engine.dispose() + + async def teardown(self) -> None: + await logger.adebug("Tearing down database") + await self.engine.dispose() diff --git a/src/lfx/src/lfx/utils/windows_postgres_helper.py b/src/lfx/src/lfx/utils/windows_postgres_helper.py new file mode 100644 index 000000000000..2b4d75b8c5dd --- /dev/null +++ b/src/lfx/src/lfx/utils/windows_postgres_helper.py @@ -0,0 +1,55 @@ +"""Helper for Windows + PostgreSQL event loop configuration. + +Relocated from ``langflow.helpers.windows_postgres_helper`` when the Tier 1 +DatabaseService moved into lfx: the base service (which now lives here) needs +this at engine-creation time, and the helper only depends on the stdlib + the +lfx logger, so it belongs in lfx. langflow re-exports it for backward +compatibility. +""" + +import asyncio +import os +import platform + +from lfx.log.logger import logger + +# The event-loop policy only matters when a Postgres URL is configured; we read +# the same env var the settings layer reads so this works before settings are +# constructed (it runs during DatabaseService.__init__). +LANGFLOW_DATABASE_URL = "LANGFLOW_DATABASE_URL" +POSTGRESQL_PREFIXES = ("postgresql", "postgres") + + +def configure_windows_postgres_event_loop(source: str | None = None) -> bool: + """Configure event loop for Windows + PostgreSQL compatibility. + + Args: + source: Optional identifier for logging context + + Returns: + True if configuration was applied, False otherwise + """ + if platform.system() != "Windows": + return False + + db_url = os.environ.get(LANGFLOW_DATABASE_URL, "") + if not db_url or not any(db_url.startswith(prefix) for prefix in POSTGRESQL_PREFIXES): + return False + + # Use getattr to safely access the Windows-only class on all platforms + selector_policy = getattr(asyncio, "WindowsSelectorEventLoopPolicy", None) + if selector_policy is None: + return False + + current_policy = asyncio.get_event_loop_policy() + if isinstance(current_policy, selector_policy): + return False + + asyncio.set_event_loop_policy(selector_policy()) + + log_context = {"event_loop": "WindowsSelectorEventLoop", "reason": "psycopg_compatibility"} + if source: + log_context["source"] = source + + logger.debug("Windows PostgreSQL event loop configured", extra=log_context) + return True diff --git a/src/lfx/tests/unit/services/database/__init__.py b/src/lfx/tests/unit/services/database/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/src/lfx/tests/unit/services/database/test_database_service.py b/src/lfx/tests/unit/services/database/test_database_service.py new file mode 100644 index 000000000000..e5efec4cbd89 --- /dev/null +++ b/src/lfx/tests/unit/services/database/test_database_service.py @@ -0,0 +1,144 @@ +"""Tests for lfx's Tier 1 DatabaseService and its own migration stream. + +These exercise the pieces that let a bare ``lfx serve`` provision and persist its +execution-history schema without langflow present: + +* the migration mirror creates exactly the lfx-owned tables under + ``lfx_alembic_version`` and does not drift from the models (``command.check``); +* the sync ``schema at head`` pre-flight the serve boot policy relies on; +* the Tier 2 ``DatabaseMemoryService`` round-trips over the real Tier 1 engine on + the migration-provisioned schema. +""" + +from __future__ import annotations + +import io +import sqlite3 +from typing import TYPE_CHECKING + +import pytest +from alembic import command +from alembic.util import exc as alembic_exc +from lfx.schema.message import Message +from lfx.services.database.service import ( + DatabaseService, + check_schema_at_head_sync, +) +from lfx.services.memory.database import DatabaseMemoryService + +if TYPE_CHECKING: + from pathlib import Path + + +@pytest.fixture +def db_settings(tmp_path, monkeypatch): + """A real settings service pointed at a throwaway sqlite database.""" + db_file = tmp_path / "lfx_test.db" + monkeypatch.setenv("LANGFLOW_DATABASE_URL", f"sqlite:///{db_file}") + monkeypatch.setenv("LANGFLOW_ALEMBIC_LOG_TO_STDOUT", "true") + + from lfx.services.settings.service import SettingsService + + settings_service = SettingsService.initialize() + # Stash the raw file path for assertions. + settings_service._test_db_file = db_file # type: ignore[attr-defined] + return settings_service + + +def _upgrade(settings_service) -> None: + cfg = DatabaseService.make_cli_config(settings_service, stdout=io.StringIO()) + command.upgrade(cfg, "head") + + +def _sqlite_tables(db_file: Path) -> set[str]: + conn = sqlite3.connect(db_file) + try: + return {r[0] for r in conn.execute("SELECT name FROM sqlite_master WHERE type='table'")} + finally: + conn.close() + + +def test_migration_mirror_creates_only_execution_tables(db_settings): + """`lfx db upgrade` provisions exactly the lfx-owned tables + its version table.""" + _upgrade(db_settings) + + tables = _sqlite_tables(db_settings._test_db_file) + assert {"message", "transaction", "vertex_build"} <= tables + assert "lfx_alembic_version" in tables + # No langflow-only tables leaked into lfx's stream. + assert "flow" not in tables + assert "user" not in tables + assert "folder" not in tables + + +def test_version_table_is_lfx_specific(db_settings): + """The lfx stream stamps its own version table, distinct from langflow's ``alembic_version``.""" + _upgrade(db_settings) + conn = sqlite3.connect(db_settings._test_db_file) + try: + rows = list(conn.execute("SELECT version_num FROM lfx_alembic_version")) + finally: + conn.close() + assert len(rows) == 1 # exactly one head stamped + assert DatabaseService.alembic_version_table == "lfx_alembic_version" + + +def test_migration_check_reports_no_drift(db_settings): + """The divergence guard: models match the migration (``command.check`` clean).""" + _upgrade(db_settings) + cfg = DatabaseService.make_cli_config(db_settings, stdout=io.StringIO()) + # Raises AutogenerateDiffsDetected if the migration lags the models. + try: + command.check(cfg) + except alembic_exc.AutogenerateDiffsDetected as exc: # pragma: no cover - failure path + pytest.fail(f"Schema drift between models and lfx migration: {exc}") + + +def test_check_schema_at_head_sync_before_and_after(db_settings): + """The fork-safe boot pre-flight flips from not-at-head to at-head after upgrade.""" + url = db_settings.settings.database_url + script = DatabaseService.default_script_location() + + at_head, current, _head = check_schema_at_head_sync( + url, script_location=script, version_table="lfx_alembic_version" + ) + assert at_head is False + assert current is None # never migrated + + _upgrade(db_settings) + + at_head, current, head = check_schema_at_head_sync(url, script_location=script, version_table="lfx_alembic_version") + assert at_head is True + assert current == head is not None + + +async def test_database_memory_persists_over_real_service(db_settings): + """Tier 2 memory round-trips over the real Tier 1 engine on the migrated schema.""" + import asyncio + + # The alembic runner calls asyncio.run() internally, so drive it off the + # test's event loop in a worker thread (same pattern DatabaseService uses). + await asyncio.to_thread(_upgrade, db_settings) + + db = DatabaseService(db_settings) + try: + memory = DatabaseMemoryService(database_service=db) + stored = await memory.aadd_messages( + [Message(text="persist me", sender="User", sender_name="alice", session_id="sess-1")] + ) + assert len(stored) == 1 + + got = await memory.aget_messages(session_id="sess-1") + assert len(got) == 1 + assert got[0].text == "persist me" + assert got[0].sender_name == "alice" + finally: + await db.teardown() + + +def test_capabilities_and_tier(): + """The real DatabaseService advertises Tier 1 + PERSISTENT.""" + from lfx.services.capabilities import Capability, Tier + + assert DatabaseService.tier == Tier.INFRASTRUCTURE + assert Capability.PERSISTENT in DatabaseService.capabilities diff --git a/uv.lock b/uv.lock index 86229c765167..285fe32d0069 100644 --- a/uv.lock +++ b/uv.lock @@ -9223,6 +9223,7 @@ dependencies = [ { name = "aiofile", version = "3.9.0", source = { registry = "https://pypi.org/simple" }, marker = "python_full_version < '3.11'" }, { name = "aiofile", version = "3.11.1", source = { registry = "https://pypi.org/simple" }, marker = "python_full_version >= '3.11'" }, { name = "aiofiles" }, + { name = "alembic" }, { name = "asyncer" }, { name = "beautifulsoup4" }, { name = "cachetools" }, @@ -9264,6 +9265,7 @@ dependencies = [ { name = "setuptools" }, { name = "sqlmodel" }, { name = "structlog" }, + { name = "tenacity" }, { name = "tomli" }, { name = "typer", version = "0.21.2", source = { registry = "https://pypi.org/simple" }, marker = "(python_full_version == '3.13.*' and platform_machine == 'arm64' and sys_platform == 'darwin') or (python_full_version < '3.12' and sys_platform == 'darwin')" }, { name = "typer", version = "0.24.2", source = { registry = "https://pypi.org/simple" }, marker = "(python_full_version >= '3.12' and platform_machine != 'arm64') or (python_full_version == '3.12.*' and platform_machine == 'arm64') or (python_full_version >= '3.14' and platform_machine == 'arm64') or sys_platform != 'darwin'" }, @@ -9316,6 +9318,7 @@ requires-dist = [ { name = "ag-ui-protocol", specifier = ">=0.1.10" }, { name = "aiofile", specifier = ">=3.8.0,<4.0.0" }, { name = "aiofiles", specifier = ">=24.1.0,<25.0.0" }, + { name = "alembic", specifier = ">=1.13.0,<2.0.0" }, { name = "asyncer", specifier = ">=0.0.8,<1.0.0" }, { name = "beautifulsoup4", specifier = ">=4.12.3,<5.0.0" }, { name = "cachetools", specifier = ">=6.0.0" }, @@ -9357,6 +9360,7 @@ requires-dist = [ { name = "setuptools", specifier = ">=80.0.0,<81.0.0" }, { name = "sqlmodel", specifier = "~=0.0.37" }, { name = "structlog", specifier = ">=25.4.0,<26.0.0" }, + { name = "tenacity", specifier = ">=8.0.0,<10.0.0" }, { name = "tomli", specifier = ">=2.2.1,<3.0.0" }, { name = "toolguard", marker = "python_full_version < '3.14' and extra == 'toolguard'", specifier = ">=0.2.20,<1.0.0" }, { name = "typer", specifier = ">=0.16.0,<1.0.0" },