mirror of
https://github.com/tiennm99/DocsGPT.git
synced 2026-10-14 04:15:35 +00:00
refactor: read settings directly instead of getattr with a second default
About 85 call sites read a setting as getattr(settings, "NAME", fallback), each carrying its own copy of the default. Every one of those names is a field with a default on the model, so the fallback could never apply to the real settings object; it only masked drift. Two had drifted: - OPENAI_PROMPT_CACHE_KEY defaults to True on the model but the reader fell back to False, and two test stubs relied on that. - SharePoint's MICROSOFT_AUTHORITY fallback to https://login.microsoftonline.com/<tenant> never fired, because the attribute always exists (as None), so MSAL got authority=None. The connector now derives the tenant authority when the setting is unset, as its test always assumed. Four places read EMBEDDINGS_KEY straight from os.environ, skipping the "None"/"" normalisation the model applies; they read the setting now. Test stubs that replaced a module's settings with a SimpleNamespace list every setting the code under test reads.
This commit is contained in:
1 parent
d70644743b
commit
f882ef49a7
46 files changed
+111
-113
No files matched your search
@@ -41,7 +41,7 @@ class SharePointAuth(BaseConnectorAuth):
|
||||
|
||||
self.redirect_uri = settings.CONNECTOR_REDIRECT_BASE_URI
|
||||
self.tenant_id = settings.MICROSOFT_TENANT_ID
|
||||
self.authority = getattr(settings, "MICROSOFT_AUTHORITY", f"https://login.microsoftonline.com/{self.tenant_id}")
|
||||
self.authority = settings.MICROSOFT_AUTHORITY or f"https://login.microsoftonline.com/{self.tenant_id}"
|
||||
|
||||
self.auth_app = ConfidentialClientApplication(
|
||||
client_id=self.client_id,
|
||||
|
||||
@@ -97,10 +97,10 @@ def bound_parse_payload(payload: Dict[str, Any], max_chars: Optional[int] = None
|
||||
|
||||
def _max_input_bytes() -> int:
|
||||
"""Return the size cap for a parsed document (its own setting, else the sandbox cap)."""
|
||||
explicit = int(getattr(settings, "DOCUMENT_PARSE_MAX_BYTES", 0) or 0)
|
||||
explicit = int(settings.DOCUMENT_PARSE_MAX_BYTES or 0)
|
||||
if explicit > 0:
|
||||
return explicit
|
||||
return int(getattr(settings, "SANDBOX_MAX_INPUT_BYTES", 25 * 1024 * 1024))
|
||||
return int(settings.SANDBOX_MAX_INPUT_BYTES)
|
||||
|
||||
|
||||
# Every zip-packaged format a parser map can route: OOXML and its macro/
|
||||
@@ -127,8 +127,8 @@ def _zip_bomb_reason(source: Union[bytes, str, Path], suffix: str) -> Optional[s
|
||||
"""
|
||||
if suffix not in _ZIP_CONTAINER_EXTENSIONS:
|
||||
return None
|
||||
max_entries = int(getattr(settings, "DOCUMENT_MAX_ARCHIVE_ENTRIES", 10000))
|
||||
cap = int(getattr(settings, "DOCUMENT_MAX_DECOMPRESSED_BYTES", 300 * 1024 * 1024))
|
||||
max_entries = int(settings.DOCUMENT_MAX_ARCHIVE_ENTRIES)
|
||||
cap = int(settings.DOCUMENT_MAX_DECOMPRESSED_BYTES)
|
||||
opened = io.BytesIO(source) if isinstance(source, bytes) else source
|
||||
try:
|
||||
with zipfile.ZipFile(opened) as zf:
|
||||
@@ -169,14 +169,14 @@ def _resolve_ocr_enabled(ocr: str) -> bool:
|
||||
return True
|
||||
if ocr == "off":
|
||||
return False
|
||||
return bool(getattr(settings, "OCR_ENABLED", False))
|
||||
return bool(settings.OCR_ENABLED)
|
||||
|
||||
|
||||
def _effective_engine(engine: str) -> str:
|
||||
"""Resolve ``auto`` to the server's ``DOC_PARSER_ENGINE``; other values pass through."""
|
||||
if engine != "auto":
|
||||
return engine
|
||||
configured = getattr(settings, "DOC_PARSER_ENGINE", None) or "anydoc"
|
||||
configured = settings.DOC_PARSER_ENGINE or "anydoc"
|
||||
return str(configured).strip().lower()
|
||||
|
||||
|
||||
|
||||
@@ -53,7 +53,7 @@ def _resolve_batch_size() -> int:
|
||||
Returns:
|
||||
Chunks per embed request, always >= 1.
|
||||
"""
|
||||
raw = getattr(settings, "EMBEDDINGS_BATCH_SIZE", None)
|
||||
raw = settings.EMBEDDINGS_BATCH_SIZE
|
||||
# Explicit type check rather than a bare ``int(raw)``: ``int(MagicMock())``
|
||||
# succeeds and yields 1, which would silently drop ingest back to the
|
||||
# per-chunk behaviour this batching replaces.
|
||||
@@ -326,7 +326,7 @@ def embed_and_store_documents(
|
||||
store = VectorCreator.create_vectorstore(
|
||||
settings.VECTOR_STORE,
|
||||
source_id=source_id,
|
||||
embeddings_key=os.getenv("EMBEDDINGS_KEY"),
|
||||
embeddings_key=settings.EMBEDDINGS_KEY,
|
||||
)
|
||||
loop_start = resume_index
|
||||
else:
|
||||
@@ -336,7 +336,7 @@ def embed_and_store_documents(
|
||||
settings.VECTOR_STORE,
|
||||
docs_init=[docs[0]],
|
||||
source_id=source_id,
|
||||
embeddings_key=os.getenv("EMBEDDINGS_KEY"),
|
||||
embeddings_key=settings.EMBEDDINGS_KEY,
|
||||
)
|
||||
# Record the seeded chunk so single-doc ingests don't fail
|
||||
# ``assert_index_complete`` — the loop never runs for
|
||||
@@ -351,7 +351,7 @@ def embed_and_store_documents(
|
||||
store = VectorCreator.create_vectorstore(
|
||||
settings.VECTOR_STORE,
|
||||
source_id=source_id,
|
||||
embeddings_key=os.getenv("EMBEDDINGS_KEY"),
|
||||
embeddings_key=settings.EMBEDDINGS_KEY,
|
||||
)
|
||||
# Only wipe the index on a fresh run — a resume must keep the
|
||||
# chunks that earlier attempts already embedded.
|
||||
|
||||
@@ -344,7 +344,7 @@ def get_default_file_extractor(
|
||||
"""
|
||||
if ocr_enabled is None:
|
||||
ocr_enabled = settings.OCR_ENABLED
|
||||
selected = (engine or getattr(settings, "DOC_PARSER_ENGINE", None) or "anydoc")
|
||||
selected = (engine or settings.DOC_PARSER_ENGINE or "anydoc")
|
||||
selected = str(selected).strip().lower()
|
||||
if selected == "docling":
|
||||
return _docling_file_extractor(ocr_enabled, pdf_text_fast_path)
|
||||
|
||||
@@ -335,11 +335,7 @@ def _ocr_min_chars_per_page() -> int:
|
||||
|
||||
try:
|
||||
return int(
|
||||
getattr(
|
||||
settings,
|
||||
"OCR_MIN_CHARS_PER_PAGE",
|
||||
_DEFAULT_OCR_MIN_CHARS_PER_PAGE,
|
||||
)
|
||||
settings.OCR_MIN_CHARS_PER_PAGE
|
||||
)
|
||||
except (TypeError, ValueError):
|
||||
return _DEFAULT_OCR_MIN_CHARS_PER_PAGE
|
||||
|
||||
@@ -124,7 +124,7 @@ def resolve_ocr_backend(requested: Optional[str] = None) -> str:
|
||||
"""
|
||||
from docsgpt.core.settings import settings
|
||||
|
||||
backend = str(requested or getattr(settings, "OCR_BACKEND", None) or "auto").strip().lower()
|
||||
backend = str(requested or settings.OCR_BACKEND or "auto").strip().lower()
|
||||
if backend not in VALID_OCR_BACKENDS:
|
||||
logger.warning(f"Unknown OCR_BACKEND {backend!r}; using auto")
|
||||
backend = "auto"
|
||||
@@ -154,7 +154,7 @@ def resolve_native_ocr_engine(requested: Optional[str] = None) -> str:
|
||||
"""
|
||||
from docsgpt.core.settings import settings
|
||||
|
||||
engine = str(requested or getattr(settings, "OCR_ENGINE", None) or "tesseract").strip().lower()
|
||||
engine = str(requested or settings.OCR_ENGINE or "tesseract").strip().lower()
|
||||
if engine in NATIVE_OCR_ENGINES:
|
||||
return engine
|
||||
if engine in VALID_OCR_ENGINES:
|
||||
@@ -172,7 +172,7 @@ def ocr_min_chars_per_page() -> int:
|
||||
from docsgpt.core.settings import settings
|
||||
|
||||
try:
|
||||
return int(getattr(settings, "OCR_MIN_CHARS_PER_PAGE", _DEFAULT_MIN_CHARS_PER_PAGE))
|
||||
return int(settings.OCR_MIN_CHARS_PER_PAGE)
|
||||
except (TypeError, ValueError):
|
||||
return _DEFAULT_MIN_CHARS_PER_PAGE
|
||||
|
||||
@@ -182,7 +182,7 @@ def render_dpi() -> int:
|
||||
from docsgpt.core.settings import settings
|
||||
|
||||
try:
|
||||
dpi = int(getattr(settings, "OCR_RENDER_DPI", _DEFAULT_RENDER_DPI))
|
||||
dpi = int(settings.OCR_RENDER_DPI)
|
||||
except (TypeError, ValueError):
|
||||
dpi = _DEFAULT_RENDER_DPI
|
||||
return max(_MIN_RENDER_DPI, min(_MAX_RENDER_DPI, dpi))
|
||||
@@ -299,7 +299,7 @@ class TesseractEngine:
|
||||
else:
|
||||
from docsgpt.core.settings import settings
|
||||
|
||||
configured = str(getattr(settings, "OCR_LANGS", "") or "eng")
|
||||
configured = str(settings.OCR_LANGS or "eng")
|
||||
langs = [lang.strip() for lang in configured.split("+") if lang.strip()]
|
||||
return "+".join(langs) or "eng"
|
||||
|
||||
@@ -388,7 +388,7 @@ class DeepseekOcrEngine:
|
||||
|
||||
self.url = url or settings.OCR_DEEPSEEK_URL
|
||||
self.model = model or settings.OCR_DEEPSEEK_MODEL
|
||||
self.timeout = float(timeout if timeout is not None else getattr(settings, "OCR_DEEPSEEK_TIMEOUT", 300))
|
||||
self.timeout = float(timeout if timeout is not None else settings.OCR_DEEPSEEK_TIMEOUT)
|
||||
self.prompt = prompt
|
||||
self.max_tokens = max_tokens
|
||||
|
||||
|
||||
@@ -129,7 +129,7 @@ class GitHubLoader(BaseRemote):
|
||||
|
||||
def _max_file_bytes(self) -> int:
|
||||
"""Resolve the per-blob size cap; ``0`` disables it."""
|
||||
raw = getattr(settings, "GITHUB_INGEST_MAX_FILE_BYTES", None)
|
||||
raw = settings.GITHUB_INGEST_MAX_FILE_BYTES
|
||||
if isinstance(raw, bool) or not isinstance(raw, (int, str)):
|
||||
return 1048576
|
||||
try:
|
||||
@@ -139,7 +139,7 @@ class GitHubLoader(BaseRemote):
|
||||
|
||||
def _max_workers(self) -> int:
|
||||
"""Resolve the parallel-fetch width, clamped to a sane range."""
|
||||
raw = getattr(settings, "GITHUB_INGEST_MAX_WORKERS", None)
|
||||
raw = settings.GITHUB_INGEST_MAX_WORKERS
|
||||
if isinstance(raw, bool) or not isinstance(raw, (int, str)):
|
||||
return 8
|
||||
try:
|
||||
|
||||
@@ -267,7 +267,7 @@ def get_token_counter(embeddings_name: Optional[str] = None) -> TokenCounter:
|
||||
A :class:`HuggingFaceCounter` for a model whose tokenizer could be
|
||||
loaded, else a :class:`TiktokenCounter`.
|
||||
"""
|
||||
name = embeddings_name or getattr(settings, "EMBEDDINGS_NAME", None)
|
||||
name = embeddings_name or settings.EMBEDDINGS_NAME
|
||||
key = name or "__default__"
|
||||
with _cache_lock:
|
||||
if key in _cache:
|
||||
|
||||
Reference in new issue
Block a user