Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 50 additions & 5 deletions packages/qdrant-loader-core/src/qdrant_loader_core/logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -357,13 +357,57 @@ def get_logger(cls, name: str | None = None) -> structlog.BoundLogger:
return structlog.get_logger(name)

@classmethod
def reconfigure(cls, *, file: str | None = None) -> None:
"""Lightweight reconfiguration for file destination.
def reconfigure(cls, *, file: str | None = None, level: str | None = None) -> None:
"""Lightweight reconfiguration for file destination and optionally log level.

Replaces only the file handler while keeping console handlers and
structlog processors intact.
structlog processors intact. Optionally updates the log level.

Args:
file: Path to log file (optional)
level: New log level (optional, e.g., "DEBUG", "INFO")
"""
root_logger = logging.getLogger()

# Update log level if provided
if level is not None:
try:
numeric_level = getattr(logging, level.upper())
root_logger.setLevel(numeric_level)

# Update structlog wrapper to use new level
if cls._current_config is not None:
_, fmt, clean_output, suppress_qdrant_warnings, disable_console = cls._current_config

# Choose timestamp format and final renderer
if clean_output and fmt == "console":
ts_fmt = "%H:%M:%S"
final_renderer = structlog.dev.ConsoleRenderer(colors=True)
else:
ts_fmt = "iso"
final_renderer = (
structlog.processors.JSONRenderer()
if fmt == "json"
else structlog.dev.ConsoleRenderer(colors=True)
)
Comment on lines +379 to +392
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Tuple unpacking mismatch will cause ValueError at runtime.

The _current_config tuple has 6 elements (level, fmt, file, clean_output, suppress_qdrant_warnings, disable_console), but line 380 unpacks into only 5 variables. This will raise ValueError: too many values to unpack.

Additionally, even if Python didn't error, the variable bindings would be incorrect—clean_output would receive the file value, etc.

Apply this diff to fix the unpacking:

                 if cls._current_config is not None:
-                    _, fmt, clean_output, suppress_qdrant_warnings, disable_console = cls._current_config
+                    _, fmt, _, clean_output, suppress_qdrant_warnings, disable_console = cls._current_config
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if cls._current_config is not None:
_, fmt, clean_output, suppress_qdrant_warnings, disable_console = cls._current_config
# Choose timestamp format and final renderer
if clean_output and fmt == "console":
ts_fmt = "%H:%M:%S"
final_renderer = structlog.dev.ConsoleRenderer(colors=True)
else:
ts_fmt = "iso"
final_renderer = (
structlog.processors.JSONRenderer()
if fmt == "json"
else structlog.dev.ConsoleRenderer(colors=True)
)
if cls._current_config is not None:
_, fmt, _, clean_output, suppress_qdrant_warnings, disable_console = cls._current_config
# Choose timestamp format and final renderer
if clean_output and fmt == "console":
ts_fmt = "%H:%M:%S"
final_renderer = structlog.dev.ConsoleRenderer(colors=True)
else:
ts_fmt = "iso"
final_renderer = (
structlog.processors.JSONRenderer()
if fmt == "json"
else structlog.dev.ConsoleRenderer(colors=True)
)
🤖 Prompt for AI Agents
In packages/qdrant-loader-core/src/qdrant_loader_core/logging.py around lines
379 to 392, the code unpacks cls._current_config into only five variables
causing a ValueError and incorrect bindings; update the unpacking to six
variables matching the stored tuple (level, fmt, file, clean_output,
suppress_qdrant_warnings, disable_console) so subsequent uses of fmt and
clean_output refer to the correct values, and ensure any later references to the
file, suppress_qdrant_warnings, or disable_console variables use these newly
unpacked names.


# Reconfigure structlog with new level
structlog.configure(
processors=[
structlog.stdlib.filter_by_level,
structlog.stdlib.add_logger_name,
structlog.stdlib.add_log_level,
structlog.processors.TimeStamper(fmt=ts_fmt),
_redact_processor,
final_renderer,
],
wrapper_class=structlog.make_filtering_bound_logger(numeric_level),
logger_factory=LoggerFactory(),
cache_logger_on_first_use=False,
)
except AttributeError:
raise ValueError(f"Invalid log level: {level}") from None
Comment on lines +373 to +409
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, let's examine the logging.py file structure and find the unpacking bug context
wc -l packages/qdrant-loader-core/src/qdrant_loader_core/logging.py

Repository: martin-papy/qdrant-loader

Length of output: 134


🏁 Script executed:

#!/bin/bash
# Read the relevant section of the file including context before line 373
sed -n '350,420p' packages/qdrant-loader-core/src/qdrant_loader_core/logging.py

Repository: martin-papy/qdrant-loader

Length of output: 3106


🏁 Script executed:

#!/bin/bash
# Search for where _current_config is set to understand the tuple structure
rg -n "_current_config\s*=" packages/qdrant-loader-core/src/qdrant_loader_core/logging.py -B2 -A2

Repository: martin-papy/qdrant-loader

Length of output: 653


🏁 Script executed:

#!/bin/bash
# Search for reconfigure calls and level parameter passing
rg -n "reconfigure\(" packages/qdrant-loader-core/src/qdrant_loader_core/logging.py -C3

Repository: martin-papy/qdrant-loader

Length of output: 405


🏁 Script executed:

#!/bin/bash
# Let's get the exact unpacking at line 382 and surrounding context
sed -n '375,390p' packages/qdrant-loader-core/src/qdrant_loader_core/logging.py | cat -n

Repository: martin-papy/qdrant-loader

Length of output: 984


🏁 Script executed:

#!/bin/bash
# Check git history for recent changes to this unpacking
cd packages/qdrant-loader-core/src/qdrant_loader_core && \
git log --oneline -20 logging.py

Repository: martin-papy/qdrant-loader

Length of output: 121


🏁 Script executed:

#!/bin/bash
# Look at git diff to see what changed in the unpacking
cd packages/qdrant-loader-core/src/qdrant_loader_core && \
git diff HEAD~5 logging.py | grep -A10 -B10 "_current_config" | head -80

Repository: martin-papy/qdrant-loader

Length of output: 255


🏁 Script executed:

#!/bin/bash
# Check the specific git commit that supposedly fixed the issue
cd packages/qdrant-loader-core/src/qdrant_loader_core && \
git show 08369cb --stat

Repository: martin-papy/qdrant-loader

Length of output: 48060


🏁 Script executed:

#!/bin/bash
# Look at the changes in that commit
cd packages/qdrant-loader-core/src/qdrant_loader_core && \
git show 08369cb | head -200

Repository: martin-papy/qdrant-loader

Length of output: 13428


🏁 Script executed:

#!/bin/bash
# Verify current state - check if unpacking matches tuple size
cd packages/qdrant-loader-core/src/qdrant_loader_core && \
python3 << 'EOF'
# Count elements in tuple structure
lines_351 = "current_tuple = (level, fmt, file, clean_output, suppress_qdrant_warnings, disable_console)"
lines_435 = "old_level, fmt, file, clean_output, suppress_qdrant_warnings, disable_console = cls._current_config"
lines_382 = "_, fmt, clean_output, suppress_qdrant_warnings, disable_console = cls._current_config"

tuple_elements_351 = 6  # level, fmt, file, clean_output, suppress_qdrant_warnings, disable_console
unpack_elements_382 = 5  # _, fmt, clean_output, suppress_qdrant_warnings, disable_console
unpack_elements_435 = 6  # old_level, fmt, file, clean_output, suppress_qdrant_warnings, disable_console

print(f"Tuple elements (line 351): {tuple_elements_351}")
print(f"Unpack elements (line 382): {unpack_elements_382}")
print(f"Unpack elements (line 435): {unpack_elements_435}")
print(f"\nLine 382 MISMATCH: {tuple_elements_351} != {unpack_elements_382}")
EOF

Repository: martin-papy/qdrant-loader

Length of output: 155


🏁 Script executed:

#!/bin/bash
# Let's verify the exact line and surrounding context one more time to be absolutely certain
sed -n '375,390p' packages/qdrant-loader-core/src/qdrant_loader_core/logging.py | cat -n

Repository: martin-papy/qdrant-loader

Length of output: 984


The tuple unpacking at line 382 is incomplete and will cause a ValueError at runtime.

The _current_config tuple contains 6 elements: (level, fmt, file, clean_output, suppress_qdrant_warnings, disable_console). Line 382 attempts to unpack only 5 elements, omitting the file element:

_, fmt, clean_output, suppress_qdrant_warnings, disable_console = cls._current_config

This will fail with ValueError: not enough values to unpack (expected 6, got 5) when level is not None. Compare with line 435 in the same method, which correctly unpacks all 6 elements:

old_level, fmt, file, clean_output, suppress_qdrant_warnings, disable_console = cls._current_config

Required fix: Update line 382 to include file in the unpacking:

_, fmt, file, clean_output, suppress_qdrant_warnings, disable_console = cls._current_config
🤖 Prompt for AI Agents
In packages/qdrant-loader-core/src/qdrant_loader_core/logging.py around lines
373 to 409, the tuple unpacking for cls._current_config on line ~382 omits the
`file` element causing a ValueError; update the unpacking to include `file` so
it reads: _, fmt, file, clean_output, suppress_qdrant_warnings, disable_console
= cls._current_config, preserving the rest of the reconfiguration logic.


# Remove existing file handler if present
if cls._file_handler is not None:
try:
Expand All @@ -386,5 +430,6 @@ def reconfigure(cls, *, file: str | None = None) -> None:

# Update current config tuple if available
if cls._current_config is not None:
level, fmt, _, clean_output, suppress_qdrant_warnings, disable_console = cls._current_config
cls._current_config = (level, fmt, file, clean_output, suppress_qdrant_warnings, disable_console)
old_level, fmt, _, clean_output, suppress_qdrant_warnings, disable_console = cls._current_config
new_level = level.upper() if level is not None else old_level
cls._current_config = (new_level, fmt, file, clean_output, suppress_qdrant_warnings, disable_console)
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def run_show_config(
)
if getattr(LoggingConfig, "reconfigure", None): # Core supports reconfigure
if getattr(LoggingConfig, "_initialized", False): # type: ignore[attr-defined]
LoggingConfig.reconfigure(file=log_file) # type: ignore[attr-defined]
LoggingConfig.reconfigure(file=log_file, level=log_level) # type: ignore[attr-defined]
else:
LoggingConfig.setup(level=log_level, format="console", file=log_file)
else:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def run_config_command(
)
if getattr(LoggingConfig, "reconfigure", None): # type: ignore[attr-defined]
if getattr(LoggingConfig, "_initialized", False): # type: ignore[attr-defined]
LoggingConfig.reconfigure(file=log_file) # type: ignore[attr-defined]
LoggingConfig.reconfigure(file=log_file, level=log_level) # type: ignore[attr-defined]
else:
LoggingConfig.setup(level=log_level, format="console", file=log_file)
else:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ async def run_ingest_command(
)
if getattr(LoggingConfig, "reconfigure", None): # type: ignore[attr-defined]
if getattr(LoggingConfig, "_initialized", False): # type: ignore[attr-defined]
LoggingConfig.reconfigure(file=log_file) # type: ignore[attr-defined]
LoggingConfig.reconfigure(file=log_file, level=log_level) # type: ignore[attr-defined]
else:
LoggingConfig.setup(level=log_level, format="console", file=log_file)
else:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ async def run_init_command(
# Setup logging first (workspace-aware later). Use core reconfigure if available.
if getattr(LoggingConfig, "reconfigure", None): # type: ignore[attr-defined]
if getattr(LoggingConfig, "_initialized", False): # type: ignore[attr-defined]
LoggingConfig.reconfigure(file="qdrant-loader.log") # type: ignore[attr-defined]
LoggingConfig.reconfigure(file="qdrant-loader.log", level=log_level) # type: ignore[attr-defined]
else:
LoggingConfig.setup(
level=log_level, format="console", file="qdrant-loader.log"
Expand Down Expand Up @@ -88,7 +88,7 @@ async def run_init_command(
else "qdrant-loader.log"
)
if getattr(LoggingConfig, "reconfigure", None): # type: ignore[attr-defined]
LoggingConfig.reconfigure(file=log_file) # type: ignore[attr-defined]
LoggingConfig.reconfigure(file=log_file, level=log_level) # type: ignore[attr-defined]
else:
import logging as _py_logging

Expand Down
Loading