[hippi345]

Motivation

Fixes #786.

RedisVL warnings never reach an application that has not configured logging, including warnings that signal lost data.

get_logger attached a logging.NullHandler whenever a logger had no handlers. On Python 3.2+, a WARNING record with no handler in the hierarchy normally goes to logging.lastResort (stderr), but a NullHandler counts as a handler, so the record was dropped. Pytest’s caplog installs its own handler, which is why tests on main did not catch this.

Notes

Loggers that already had a NullHandler attached earlier in the same process keep it; this change does not remove third-party handlers.

Release Notes

RedisVL warnings and errors now show on stderr when the application has not configured logging.

Reviewer Brief

What changed and why

Fixes #786 by removing NullHandler from get_logger and not attaching any custom handler. With no handler on the logger, Python’s lastResort prints WARNING and above to stderr when the application has not configured logging—the behaviour the stdlib documents for libraries.

The earlier _DefaultStderrHandler stack is gone (per maintainer review): it reimplemented lastResort, risked lock-order deadlocks with dictConfig, and diverged from plain logging.getLogger call sites. CLI failure output is fixed at the call site: rvl index, rvl stats, and rvl migrate no longer call logger.error(..., exc_info=True) where they already print the same error, so stderr stays a single line without a traceback.

Docs: docs/concepts/queries.md no longer says RedisVL “silently skips” empty field payloads; the WARNING is visible on stderr when logging is unconfigured.

Files to focus on

File	Why
redisvl/utils/log.py	Entire behavioural change; should be ~10 lines.
redisvl/cli/index.py, stats.py, migrate.py	Exception handlers: stderr print only.
tests/unit/test_log.py	Two subprocess tests pin #786 (unconfigured vs basicConfig).
tests/unit/test_cli_index.py	test_index_failure_stderr_subprocess_has_no_traceback pins CLI stderr shape.
docs/concepts/queries.md	User-visible wording for WARNING on stderr.

How to test

make lint
make test
# Minimum focused run:
pytest tests/unit/test_log.py \
  tests/unit/test_cli_index.py::test_index_failure_stderr_subprocess_has_no_traceback -v

Manual sanity (Python 3.11+): in a fresh interpreter with no logging.basicConfig, from redisvl.utils.log import get_logger; get_logger("redisvl.demo").warning("hi") should print one line on stderr; .info(...) should not. After basicConfig(level=logging.WARNING), the same warning should appear once on the configured stream.

Risk

• Low: Loggers that already have a third-party NullHandler in the same process stay silenced (documented in PR Notes).
• Low: Library code that logs exc_info=True on an unconfigured app will show a traceback via lastResort (stdlib); MCP modules already use plain get_logger—consistent now.
• CLI: No structured log on failure when the host app has configured logging (only stderr print). Acceptable for rvl; embedders can wrap the CLI if they need both.

---

Start here: redisvl/utils/log.py, get_logger. Confirm it only resolves the debug name alias and returns logging.getLogger(name) with no handler attached.

Where it could fail

• redisvl/cli/index.py:83 (and the matching handlers in stats.py / migrate.py): a future change that adds logger.error(..., exc_info=True) back alongside print would restore duplicate tracebacks on unconfigured runs.
• docs/concepts/queries.md:357: user docs must stay aligned with WARNING reaching stderr when logging is not configured.

Not verified: Full make test in an environment without Docker (session redis_container fixture). Cluster mode and Python versions outside 3.12.3 in the review environment.

Beyond CI: make lint passed (black/isort unchanged after format, mypy clean on 122 files). Subprocess regression tests: tests/unit/test_log.py — 2 passed; test_index_failure_stderr_subprocess_has_no_traceback — passed; all 7 test_index_runtime_errors parametrized cases — passed (--confcutdir=tests/unit where Redis fixtures are unavailable). On origin/main, the unconfigured-stderr subprocess reproduction omits the RedisVL warning line; those two test_log.py cases fail there.

Since the last brief: 4264785 — replaced _DefaultStderrHandler / log_cli_exception with the #786-style bare logger and CLI print-only errors; trimmed tests and updated queries doc per review.

_{Reviewed 4264785 · Cursor agent (composer-2.5) · lenses: correctness, security, design, tests, docs, Redis behaviour}

[cursor]

Stale Bugbot comment from a previous run.

[vishal-bala]

Hi, please follow the current contribution guidelines and include a "Reviewer Brief" in your description (based on a pre-PR review using the skill included in the repository).

[cursor]

Stale Bugbot comment from a previous run.

[hippi345]

good catch, dropped the propagate=False and the default handler now only kicks in when the app hasn't set up its own logging. added tests for basicConfig and a parent logger handler. fixed in da3b4c1.

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

^{Reviewed by Cursor Bugbot for commit da3b4c1. Configure here.}

[vishal-bala]

Hi, your reviewer brief doesn't meet the expectations that the repository and the skill set out. If you're using an agent, it should be clear enough for them to run the subagent review panel with the lenses expected.

When your PR has gone through the proper preparation, we can have a human reviewer take a look at it 👍

[hippi345]

Hi @vishal-bala — thanks for calling out the earlier brief. I re-ran preparation against head ba9f127: full pre-PR review, make lint, and the touched unit tests. Lint and mypy are clean; all 8 tests in tests/unit/test_log.py and all 7 test_index_runtime_errors cases pass locally (--confcutdir=tests/unit here because this environment has no Docker; CI still uses the normal conftest path). I also checked origin/main: the subprocess reproduction omits the RedisVL warning, and the new subprocess tests fail there on the three unconfigured-stderr cases. The PR description now follows the template with an updated Reviewer Brief. Ready for a human pass when you are.

[vishal-bala]

Could this go back to the fix proposed in #786, which drops the NullHandler and attaches nothing? With no handler on a RedisVL logger, the stdlib's lastResort already does everything this handler re-creates. I measured both on 3.13:

• INFO stays hidden.
• A child warning and a parent warning each print once.
• A root or parent handler takes over once the app configures logging.
• An existing NullHandler still silences the logger.

The plain fix leaves one problem, in the CLI (see the comment on cli/utils.py). Its cause is that each call site both logs and prints the same error, so it's better fixed there. A handler like this one is something every later logging change has to reason about, and it adds costs of its own:

• It can deadlock. emit() runs while the handler's lock is held, and it calls logging.getLogger(), which takes logging._lock. dictConfig takes the two locks in the opposite order: it takes logging._lock first, then acquires every handler's lock in _clearExistingHandlers(). So a RedisVL warning on one thread and a dictConfig call on another can block each other for good. I reproduced it on 3.13.9 by adding a sleep in emit() to widen the window.
• Two different unconfigured behaviours. Because the handler strips exc_info, get_logger loggers behave differently from the plain logging.getLogger loggers in redisvl/mcp/, which keep their tracebacks.
• Against stdlib guidance. The Python logging HOWTO tells libraries to add no handler other than NullHandler, and to leave output to the application.

get_logger would then be:

def get_logger(name, log_level="info", fmt=None):
    """Return a logger instance."""
    name = "RedisVL" if log_level == "debug" else name
    return logging.getLogger(name)

One docs line also needs updating. docs/concepts/queries.md:357 says RedisVL "silently skips" a document whose fields came back empty, while it logs a WARNING. That warning now reaches stderr.

[vishal-bala]

log_cli_exception never logs in a real rvl process. Nothing under redisvl/ configures logging, so _has_application_handlers("[RedisVL]") is always False there (measured). It only fires when the CLI classes run inside a process that already has handlers, such as pytest.

Could the three call sites just drop their log call instead?

• In cli/index.py:86 and cli/stats.py:116, delete logger.error(e, exc_info=True) and keep the print.
• In cli/migrate.py:77, delete logger.error(e).

Once the NullHandler is gone, that gives the same single error line this PR prints, without importing a private helper from log.py.

[vishal-bala]

This assertion passes on main as well. Under pytest the root logger holds caplog's handlers, so the record never reaches stderr, whatever log_cli_exception does. Could the CLI case be a subprocess test instead? It would run a failing command with logging unconfigured and assert that stderr has one error line and no Traceback.

[vishal-bala]

Once the handler is gone, most of this file can go too. Two subprocess tests would pin #786:

• With logging unconfigured, a WARNING reaches stderr and an INFO doesn't.
• After basicConfig(), the warning appears exactly once.

The tests that import _DefaultStderrHandler or count handlers check implementation details rather than behaviour.

[hippi345]

@vishal-bala I dropped the custom handler stack and went back to the bare get_logger (no NullHandler), removed the extra logger.error calls in the CLI where we already print to stderr, trimmed test_log.py to the two subprocess cases you suggested, added a subprocess test for rvl index instead of the caplog traceback check, and fixed the queries doc line (4264785). The Reviewer Brief in the description is updated. make lint is clean and the new log/CLI tests pass locally.