refactor(documents): fully decouple document stack from server startup; Windows-safe tests
Addresses round-1 review on #878: - Move the eager `document_processors` imports out of the API startup graph: `app.py` (get_registry now imported inside initialize_document_processors, after the disabled early-return) and `vector/processor.py` (get_registry now imported at its single use site). Importing `app` + `cli` no longer loads `document_processors` / `_isolation` at all -- the #877 stack is fully out of startup (pymupdf still loads via search/pdf_highlighter, a Windows-compatible and separately-tracked concern). - Make `tests/unit/test_pdf_parse_isolation.py` importable on Windows: guard the top-level `import resource` with try/except and skip the three rlimit computation tests via a `requires_resource` marker when the module is absent. The Windows no-op / import-guard tests don't use the real module and still run. - Fix the `# pragma: no cover` comment on the win32 branch to be accurate. - Add `enable-cache: true` to the package-smoke setup-uv step. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
fc8a4e4dfa
commit
62274069de
@@ -47,6 +47,8 @@ jobs:
|
|||||||
- uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
|
- uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
|
||||||
- name: Install the latest version of uv
|
- name: Install the latest version of uv
|
||||||
uses: astral-sh/setup-uv@37802adc94f370d6bfd71619e3f0bf239e1f3b78 # v7.6.0
|
uses: astral-sh/setup-uv@37802adc94f370d6bfd71619e3f0bf239e1f3b78 # v7.6.0
|
||||||
|
with:
|
||||||
|
enable-cache: true
|
||||||
- name: Smoke test CLI (isolated install)
|
- name: Smoke test CLI (isolated install)
|
||||||
run: uv run --isolated --no-project --with . nextcloud-mcp-server --help
|
run: uv run --isolated --no-project --with . nextcloud-mcp-server --help
|
||||||
|
|
||||||
|
|||||||
@@ -109,7 +109,6 @@ from nextcloud_mcp_server.config_validators import (
|
|||||||
validate_configuration,
|
validate_configuration,
|
||||||
)
|
)
|
||||||
from nextcloud_mcp_server.context import get_client as get_nextcloud_client
|
from nextcloud_mcp_server.context import get_client as get_nextcloud_client
|
||||||
from nextcloud_mcp_server.document_processors import get_registry
|
|
||||||
from nextcloud_mcp_server.http import nextcloud_httpx_client
|
from nextcloud_mcp_server.http import nextcloud_httpx_client
|
||||||
from nextcloud_mcp_server.observability import (
|
from nextcloud_mcp_server.observability import (
|
||||||
ObservabilityMiddleware,
|
ObservabilityMiddleware,
|
||||||
@@ -159,6 +158,11 @@ def initialize_document_processors():
|
|||||||
logger.info("Document processing disabled")
|
logger.info("Document processing disabled")
|
||||||
return
|
return
|
||||||
|
|
||||||
|
# Imported lazily so the API startup path never loads the ingest document
|
||||||
|
# stack (document_processors -> pymupdf -> _isolation) unless document
|
||||||
|
# processing is actually enabled -- see #877 / the API-vs-ingest split.
|
||||||
|
from nextcloud_mcp_server.document_processors import get_registry # noqa: PLC0415
|
||||||
|
|
||||||
registry = get_registry()
|
registry = get_registry()
|
||||||
registered_count = 0
|
registered_count = 0
|
||||||
|
|
||||||
|
|||||||
@@ -25,7 +25,7 @@ from anyio import BrokenWorkerProcess
|
|||||||
# importing it unconditionally crashed Windows startup (#877). The RLIMIT_AS cap
|
# importing it unconditionally crashed Windows startup (#877). The RLIMIT_AS cap
|
||||||
# it provides is a Linux-pod safety measure, not a correctness requirement, so on
|
# it provides is a Linux-pod safety measure, not a correctness requirement, so on
|
||||||
# platforms without it we fall back to a no-op (``resource is None``).
|
# platforms without it we fall back to a no-op (``resource is None``).
|
||||||
if sys.platform == "win32": # pragma: no cover - exercised only on Windows
|
if sys.platform == "win32": # pragma: no cover - win32-only path
|
||||||
resource = None
|
resource = None
|
||||||
else:
|
else:
|
||||||
import resource
|
import resource
|
||||||
|
|||||||
@@ -16,7 +16,6 @@ from qdrant_client.models import PointStruct
|
|||||||
from nextcloud_mcp_server.acl_hash import compute_acl_hash
|
from nextcloud_mcp_server.acl_hash import compute_acl_hash
|
||||||
from nextcloud_mcp_server.client import NextcloudClient
|
from nextcloud_mcp_server.client import NextcloudClient
|
||||||
from nextcloud_mcp_server.config import get_settings
|
from nextcloud_mcp_server.config import get_settings
|
||||||
from nextcloud_mcp_server.document_processors import get_registry
|
|
||||||
from nextcloud_mcp_server.embedding import get_bm25_service, get_embedding_service
|
from nextcloud_mcp_server.embedding import get_bm25_service, get_embedding_service
|
||||||
from nextcloud_mcp_server.models.deck import DeckCard
|
from nextcloud_mcp_server.models.deck import DeckCard
|
||||||
from nextcloud_mcp_server.observability.metrics import (
|
from nextcloud_mcp_server.observability.metrics import (
|
||||||
@@ -628,6 +627,12 @@ async def _index_document(
|
|||||||
):
|
):
|
||||||
# The registry runs the tiered PDF pipeline (tier-0 classify ->
|
# The registry runs the tiered PDF pipeline (tier-0 classify ->
|
||||||
# tier-1 fast -> OCR escalation) and records classification metrics.
|
# tier-1 fast -> OCR escalation) and records classification metrics.
|
||||||
|
# Imported lazily so module import doesn't pull in the document stack
|
||||||
|
# (document_processors -> _isolation, Unix-only ``resource``; see #877).
|
||||||
|
from nextcloud_mcp_server.document_processors import ( # noqa: PLC0415
|
||||||
|
get_registry,
|
||||||
|
)
|
||||||
|
|
||||||
registry = get_registry()
|
registry = get_registry()
|
||||||
|
|
||||||
try:
|
try:
|
||||||
|
|||||||
@@ -13,7 +13,6 @@ check on the sample PDFs, not here (unit tests must not spawn the heavy worker
|
|||||||
or depend on the sample files).
|
or depend on the sample files).
|
||||||
"""
|
"""
|
||||||
|
|
||||||
import resource
|
|
||||||
import sys
|
import sys
|
||||||
|
|
||||||
import anyio
|
import anyio
|
||||||
@@ -28,8 +27,20 @@ from nextcloud_mcp_server.document_processors._isolation import (
|
|||||||
run_isolated_pdf_parse,
|
run_isolated_pdf_parse,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# ``resource`` is a Unix-only stdlib module (absent on Windows, #877). Guard the
|
||||||
|
# import so this test module stays importable on Windows; the rlimit-computation
|
||||||
|
# tests below are skipped there via the ``requires_resource`` marker.
|
||||||
|
try:
|
||||||
|
import resource
|
||||||
|
except ImportError: # pragma: no cover - only reached on Windows
|
||||||
|
resource = None # type: ignore[assignment]
|
||||||
|
|
||||||
pytestmark = pytest.mark.unit
|
pytestmark = pytest.mark.unit
|
||||||
|
|
||||||
|
requires_resource = pytest.mark.skipif(
|
||||||
|
resource is None, reason="resource module is Unix-only (absent on Windows)"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def _tiny_pdf() -> bytes:
|
def _tiny_pdf() -> bytes:
|
||||||
doc = pymupdf.open()
|
doc = pymupdf.open()
|
||||||
@@ -112,6 +123,7 @@ async def test_timeout_kills_and_classifies_as_timeout(monkeypatch):
|
|||||||
# --- _apply_mem_limit computation (mocked; never applied to the test proc) ---
|
# --- _apply_mem_limit computation (mocked; never applied to the test proc) ---
|
||||||
|
|
||||||
|
|
||||||
|
@requires_resource
|
||||||
def test_apply_mem_limit_caps_soft_below_finite_hard(monkeypatch):
|
def test_apply_mem_limit_caps_soft_below_finite_hard(monkeypatch):
|
||||||
captured = {}
|
captured = {}
|
||||||
monkeypatch.setattr(_isolation, "_MEM_LIMIT_APPLIED", False)
|
monkeypatch.setattr(_isolation, "_MEM_LIMIT_APPLIED", False)
|
||||||
@@ -130,6 +142,7 @@ def test_apply_mem_limit_caps_soft_below_finite_hard(monkeypatch):
|
|||||||
assert hard == 4 * 1024**3
|
assert hard == 4 * 1024**3
|
||||||
|
|
||||||
|
|
||||||
|
@requires_resource
|
||||||
def test_apply_mem_limit_uses_target_when_hard_unlimited(monkeypatch):
|
def test_apply_mem_limit_uses_target_when_hard_unlimited(monkeypatch):
|
||||||
captured = {}
|
captured = {}
|
||||||
monkeypatch.setattr(_isolation, "_MEM_LIMIT_APPLIED", False)
|
monkeypatch.setattr(_isolation, "_MEM_LIMIT_APPLIED", False)
|
||||||
@@ -148,6 +161,7 @@ def test_apply_mem_limit_uses_target_when_hard_unlimited(monkeypatch):
|
|||||||
assert hard == resource.RLIM_INFINITY
|
assert hard == resource.RLIM_INFINITY
|
||||||
|
|
||||||
|
|
||||||
|
@requires_resource
|
||||||
def test_apply_mem_limit_is_applied_once(monkeypatch):
|
def test_apply_mem_limit_is_applied_once(monkeypatch):
|
||||||
calls = []
|
calls = []
|
||||||
monkeypatch.setattr(_isolation, "_MEM_LIMIT_APPLIED", False)
|
monkeypatch.setattr(_isolation, "_MEM_LIMIT_APPLIED", False)
|
||||||
|
|||||||
Reference in New Issue
Block a user