Skip to content

Populate KeyValue Entry.created from message timestamp - #989

Open
caspervonb wants to merge 2 commits into
mainfrom
fix-398
Open

Populate KeyValue Entry.created from message timestamp#989
caspervonb wants to merge 2 commits into
mainfrom
fix-398

Conversation

@caspervonb

Copy link
Copy Markdown
Collaborator

KeyValue.get() hardcoded Entry.created to None, so callers had no creation time for a fetched key. Both the direct-get and API-get paths already carry the timestamp on RawStreamMsg.time; use it.

Fixes #398.

KeyValue.get() hardcoded Entry.created to None, so callers had no
creation time for a fetched key and purge_deletes-style age math on a
fetched entry was impossible. Both the direct-get and API-get paths
already carry the message timestamp on RawStreamMsg.time, so use it.
@caspervonb caspervonb added the nats-legacy Legacy asyncio NATS client label Jul 14, 2026
@synadia-claude-reviewer

synadia-claude-reviewer Bot commented Jul 14, 2026

Copy link
Copy Markdown

Claude finished @caspervonb's task in 1m 21s —— View job


PR Review

  • Gather context
  • Read changed files
  • Review diff
  • Post findings

The fix is correct. _get() now passes msg.time (a datetime.datetime parsed from the server's ISO 8601 timestamp) into Entry.created for both the direct-get (direct=True) and API-get (direct=False) paths. This matches what the watcher path has always done (created=meta.timestamp in watch_updates).

One pre-existing issue worth fixing here: Entry.created is declared as Optional[int] at kv.py:95, but in practice it always holds a datetime.datetime (or None). Both the watcher path and this newly-fixed path store datetimes, and purge_deletes() already performs datetime arithmetic on it (entry.created subtracted from datetime.datetime.now(...)). The PR makes this mismatch more visible — worth correcting the annotation in the same change:

# kv.py line 95 — fix the annotation
created: Optional[datetime.datetime]

Test coverage is good: the new test exercises both direct=True and direct=False paths and asserts the type explicitly.

@caspervonb
caspervonb marked this pull request as ready for review August 28, 2026 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

nats-legacy Legacy asyncio NATS client

Projects

None yet

Development

Successfully merging this pull request may close these issues.

KeyValue creation time not returned when using get method

1 participant