Skip to content

Honor ConsumerConfig.durable_name in pull_subscribe - #992

Draft
caspervonb wants to merge 2 commits into
mainfrom
fix-603
Draft

Honor ConsumerConfig.durable_name in pull_subscribe#992
caspervonb wants to merge 2 commits into
mainfrom
fix-603

Conversation

@caspervonb

Copy link
Copy Markdown
Collaborator

When the durable was supplied only through the config, pull_subscribe generated an ephemeral name while leaving durable_name set, so the server rejected the mismatched request. Resolve the durable from config.durable_name before the lookup.

Fixes #603.

When durable was passed only through the config (not the durable
argument), pull_subscribe generated an ephemeral name while leaving the
user's durable_name set, so the server rejected the mismatched request.
Resolve the durable from config.durable_name before the lookup so it is
created and bound as a durable.
@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 27s —— View job


Code Review

  • Gather context (reading PR diff and source files)
  • Analyze changes for correctness, security, and performance
  • Post inline comments on specific issues
  • Summarize findings

The fix is correct and minimal. The root cause was clear: pull_subscribe never consulted config.durable_name when the durable positional argument was omitted, so it fell through to generate an ephemeral consumer_name at line 611, producing a name mismatch that the server rejected.

The placement of the new check (after stream lookup, before the should_create block) is exactly right, and config.durable_name as a truthiness check safely covers both None and empty string.

One minor gap in the test: it only verifies the consumer exists after a single subscribe call. It doesn't verify that calling pull_subscribe a second time with the same config re-binds to the existing consumer (i.e. should_create = False) rather than erroring or creating a second one. This is the behavior most likely to regress silently, since the whole point of a durable is idempotent reuse. Worth adding a second pull_subscribe call and asserting the same consumer info is returned.

No correctness bugs, no security or async pattern issues. The fix is ready to merge as-is; the second test case would be a nice addition but is not blocking.

Comment thread nats/tests/test_js.py
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.

JetStreamContext.subscribe() and JetStreamContext.pull_subscribe() ignore "durable_name" from ConsumerConfig when looking up for consumer info.

1 participant