-
Notifications
You must be signed in to change notification settings - Fork 13
fix: log level not reconfigurable at runtime Fix #58 #63
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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) | ||
| ) | ||
|
|
||
| # 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 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.pyRepository: 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.pyRepository: 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 -A2Repository: 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 -C3Repository: 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 -nRepository: 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.pyRepository: 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 -80Repository: 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 --statRepository: 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 -200Repository: 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}")
EOFRepository: 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 -nRepository: martin-papy/qdrant-loader Length of output: 984 The tuple unpacking at line 382 is incomplete and will cause a ValueError at runtime. The _, fmt, clean_output, suppress_qdrant_warnings, disable_console = cls._current_configThis will fail with old_level, fmt, file, clean_output, suppress_qdrant_warnings, disable_console = cls._current_configRequired fix: Update line 382 to include _, fmt, file, clean_output, suppress_qdrant_warnings, disable_console = cls._current_config🤖 Prompt for AI Agents |
||
|
|
||
| # Remove existing file handler if present | ||
| if cls._file_handler is not None: | ||
| try: | ||
|
|
@@ -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) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Tuple unpacking mismatch will cause
ValueErrorat runtime.The
_current_configtuple has 6 elements (level, fmt, file, clean_output, suppress_qdrant_warnings, disable_console), but line 380 unpacks into only 5 variables. This will raiseValueError: too many values to unpack.Additionally, even if Python didn't error, the variable bindings would be incorrect—
clean_outputwould receive thefilevalue, etc.Apply this diff to fix the unpacking:
📝 Committable suggestion
🤖 Prompt for AI Agents