Merge pull request #797 from cbcoutinho/fix/bg-ops-advisory-log-once
fix(config): emit background-ops advisory logs once per process
This commit is contained in:
@@ -819,23 +819,25 @@ def _is_multi_user_mode() -> bool:
|
||||
return True
|
||||
|
||||
|
||||
def _get_background_operations_enabled() -> bool:
|
||||
"""Get background operations enabled status with auto-enablement for semantic search.
|
||||
# Per-process guard for the three advisory log messages emitted by
|
||||
# `_get_background_operations_enabled()`. The function runs on every
|
||||
# `get_settings()` call (per ADR-024 / dynaconf design `get_settings()` is
|
||||
# intentionally non-cached), so unguarded `logger.info`/`logger.warning`
|
||||
# calls spam every MCP tool invocation. Mirrors the precedent at
|
||||
# `nextcloud_mcp_server/vector/webhook_receiver.py:_warn_missing_secret_once`.
|
||||
_bg_ops_advisories_logged: bool = False
|
||||
|
||||
Supports:
|
||||
- ENABLE_BACKGROUND_OPERATIONS (new, preferred)
|
||||
- ENABLE_OFFLINE_ACCESS (old, deprecated)
|
||||
- Auto-enabled if ENABLE_SEMANTIC_SEARCH=true in multi-user modes
|
||||
|
||||
Returns:
|
||||
True if background operations should be enabled
|
||||
"""
|
||||
def _log_bg_ops_advisories_once(
|
||||
explicit: bool, legacy: bool, auto_enabled: bool
|
||||
) -> None:
|
||||
"""Emit ENABLE_BACKGROUND_OPERATIONS advisory logs at most once per process."""
|
||||
global _bg_ops_advisories_logged
|
||||
if _bg_ops_advisories_logged:
|
||||
return
|
||||
_bg_ops_advisories_logged = True
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
# Check new and old variable names
|
||||
explicit = _dynaconf.get("ENABLE_BACKGROUND_OPERATIONS", False)
|
||||
legacy = _dynaconf.get("ENABLE_OFFLINE_ACCESS", False)
|
||||
|
||||
if explicit and legacy:
|
||||
logger.warning(
|
||||
"Both ENABLE_BACKGROUND_OPERATIONS and ENABLE_OFFLINE_ACCESS are set. "
|
||||
@@ -848,18 +850,32 @@ def _get_background_operations_enabled() -> bool:
|
||||
"Please use ENABLE_BACKGROUND_OPERATIONS instead. "
|
||||
"Support for ENABLE_OFFLINE_ACCESS will be removed in v1.0.0."
|
||||
)
|
||||
|
||||
# Auto-enable if semantic search is enabled in multi-user mode
|
||||
semantic_search_enabled = _get_semantic_search_enabled()
|
||||
is_multi_user = _is_multi_user_mode()
|
||||
auto_enabled = semantic_search_enabled and is_multi_user
|
||||
|
||||
if auto_enabled and not (explicit or legacy):
|
||||
logger.info(
|
||||
"Automatically enabled background operations for semantic search in multi-user mode. "
|
||||
"Set ENABLE_BACKGROUND_OPERATIONS=false to disable (this will also disable semantic search)."
|
||||
)
|
||||
|
||||
|
||||
def _get_background_operations_enabled() -> bool:
|
||||
"""Get background operations enabled status with auto-enablement for semantic search.
|
||||
|
||||
Supports:
|
||||
- ENABLE_BACKGROUND_OPERATIONS (new, preferred)
|
||||
- ENABLE_OFFLINE_ACCESS (old, deprecated)
|
||||
- Auto-enabled if ENABLE_SEMANTIC_SEARCH=true in multi-user modes
|
||||
|
||||
Returns:
|
||||
True if background operations should be enabled
|
||||
"""
|
||||
explicit = _dynaconf.get("ENABLE_BACKGROUND_OPERATIONS", False)
|
||||
legacy = _dynaconf.get("ENABLE_OFFLINE_ACCESS", False)
|
||||
semantic_search_enabled = _get_semantic_search_enabled()
|
||||
is_multi_user = _is_multi_user_mode()
|
||||
auto_enabled = semantic_search_enabled and is_multi_user
|
||||
|
||||
_log_bg_ops_advisories_once(explicit, legacy, auto_enabled)
|
||||
|
||||
return explicit or legacy or auto_enabled
|
||||
|
||||
|
||||
|
||||
@@ -16,6 +16,7 @@ def _reload_dynaconf_after_test():
|
||||
validation should call _reload_config() explicitly.
|
||||
"""
|
||||
yield
|
||||
from nextcloud_mcp_server.config import _dynaconf
|
||||
from nextcloud_mcp_server import config as _config
|
||||
|
||||
_dynaconf.reload()
|
||||
_config._dynaconf.reload()
|
||||
_config._bg_ops_advisories_logged = False
|
||||
|
||||
@@ -698,6 +698,85 @@ class TestConfigurationConsolidation:
|
||||
# Verify background operations were auto-enabled
|
||||
assert settings.enable_offline_access is True
|
||||
|
||||
def test_auto_enable_info_log_emitted_at_most_once(self, caplog):
|
||||
"""Auto-enable INFO advisory must fire once per process, not per get_settings() call.
|
||||
|
||||
Regression: `get_settings()` is non-cached and called per-request from
|
||||
`get_client()`, so unguarded `logger.info` calls in
|
||||
`_get_background_operations_enabled()` spammed every MCP tool invocation
|
||||
(observed: 569 entries/hour in tenant `tenant-e2e-disc-0033`).
|
||||
"""
|
||||
import logging
|
||||
|
||||
with patch.dict(
|
||||
os.environ,
|
||||
{
|
||||
"NEXTCLOUD_HOST": "http://localhost:8080",
|
||||
"ENABLE_SEMANTIC_SEARCH": "true",
|
||||
"QDRANT_LOCATION": ":memory:",
|
||||
"TOKEN_ENCRYPTION_KEY": "test-key",
|
||||
"TOKEN_STORAGE_DB": "/tmp/test.db",
|
||||
# No NEXTCLOUD_USERNAME/PASSWORD → multi-user mode → auto-enable triggers
|
||||
},
|
||||
clear=True,
|
||||
):
|
||||
from nextcloud_mcp_server.config import get_settings
|
||||
|
||||
_reload_config()
|
||||
caplog.set_level(logging.INFO, logger="nextcloud_mcp_server.config")
|
||||
|
||||
for _ in range(5):
|
||||
settings = get_settings()
|
||||
assert settings.enable_offline_access is True
|
||||
|
||||
auto_enable_records = [
|
||||
r
|
||||
for r in caplog.records
|
||||
if r.name == "nextcloud_mcp_server.config"
|
||||
and "Automatically enabled background operations" in r.message
|
||||
]
|
||||
assert len(auto_enable_records) == 1, (
|
||||
f"Expected exactly one auto-enable advisory log, "
|
||||
f"got {len(auto_enable_records)}: "
|
||||
f"{[r.message for r in auto_enable_records]}"
|
||||
)
|
||||
|
||||
def test_legacy_offline_access_deprecation_warning_emitted_at_most_once(
|
||||
self, caplog
|
||||
):
|
||||
"""Legacy `ENABLE_OFFLINE_ACCESS` deprecation WARNING is also one-shot."""
|
||||
import logging
|
||||
|
||||
with patch.dict(
|
||||
os.environ,
|
||||
{
|
||||
"NEXTCLOUD_HOST": "http://localhost:8080",
|
||||
"ENABLE_OFFLINE_ACCESS": "true",
|
||||
"TOKEN_ENCRYPTION_KEY": "test-key",
|
||||
"TOKEN_STORAGE_DB": "/tmp/test.db",
|
||||
},
|
||||
clear=True,
|
||||
):
|
||||
from nextcloud_mcp_server.config import get_settings
|
||||
|
||||
_reload_config()
|
||||
caplog.set_level(logging.WARNING, logger="nextcloud_mcp_server.config")
|
||||
|
||||
for _ in range(5):
|
||||
get_settings()
|
||||
|
||||
deprecation_records = [
|
||||
r
|
||||
for r in caplog.records
|
||||
if r.name == "nextcloud_mcp_server.config"
|
||||
and "ENABLE_OFFLINE_ACCESS is deprecated" in r.message
|
||||
]
|
||||
assert len(deprecation_records) == 1, (
|
||||
f"Expected exactly one deprecation warning, "
|
||||
f"got {len(deprecation_records)}: "
|
||||
f"{[r.message for r in deprecation_records]}"
|
||||
)
|
||||
|
||||
|
||||
class TestExplicitModeSelection:
|
||||
"""Test ADR-021 explicit mode selection via MCP_DEPLOYMENT_MODE.
|
||||
|
||||
Reference in New Issue
Block a user