Skip to content

docs(logging_observer): fix and strengthen create_packet_logger Warning#102

Merged
eywalker merged 2 commits intodevfrom
eywalker/eng-299-logging-observer-docstring
Mar 24, 2026
Merged

docs(logging_observer): fix and strengthen create_packet_logger Warning#102
eywalker merged 2 commits intodevfrom
eywalker/eng-299-logging-observer-docstring

Conversation

@kurodo3
Copy link
Copy Markdown
Contributor

@kurodo3 kurodo3 Bot commented Mar 24, 2026

Summary

  • Corrects an inaccurate Note in LoggingObserver.create_packet_logger: it said node_label/node_hash default to "unknown" when called on the root observer; the actual default is "" (empty string).
  • Upgrades the note from a soft Note: to a Warning:, making the misuse consequence explicit.
  • Adds a concrete correct-usage example showing that contextualize() must be called first, consistent with how FunctionNode.execute uses the API.

Context (ENG-299)

ENG-299 traced empty _log_node_label/_log_node_hash columns to callers invoking create_packet_logger on a non-contextualized observer. The docstring previously downplayed this as a note with wrong default values, which made the constraint easy to miss. This PR makes the contract clear and accurate.

The root cause fix (missing contextualize() calls in portolan/tests/conftest.py) is addressed via a PR review comment on nauticalab/portolan#5.

Test plan

  • All existing pipeline tests pass (uv run pytest tests/test_pipeline/ — 328 passed)
  • Docstring accurately describes the "" default and the mandatory contextualize() call

Relates to ENG-299.

🤖 Generated with Claude Code

The previous Note said node_label/node_hash default to "unknown" when
called on a non-contextualized observer; the actual default is "" (empty
string). Also upgrades the Note to a Warning and adds a concrete correct-
usage example showing that contextualize() must be called first, mirroring
how FunctionNode.execute uses the API.

Relates to ENG-299.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Replace every RST cross-reference role (:class:, :meth:, :func:, :exc:,
:attr:, ~-prefixed names) and directive (.. note::) with their Google-style
equivalents (plain backtick names, Note: sections) across 11 source files.

No logic changes; package imports and all 328 tests pass unchanged.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@eywalker eywalker merged commit 63e9b54 into dev Mar 24, 2026
5 checks passed
@eywalker eywalker deleted the eywalker/eng-299-logging-observer-docstring branch March 24, 2026 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant