Add nats-client package #732
Conversation
caspervonb
force-pushed
the
add-nats-client-package
branch
from
381af5b to
781a9bf
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new NATS client package with significant performance improvements over the existing nats.aio client. The implementation provides core NATS messaging functionality including publish/subscribe, request/reply, queue groups, and message headers through an asyncio-based Python client.
- Introduces a complete NATS client implementation with high-level API
- Adds comprehensive test coverage for all client features and edge cases
- Includes performance benchmarking tools demonstrating 35x improvement for small messages
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| nats-client/src/nats/client/init.py | Main client implementation with connection management, messaging, and reconnection logic |
| nats-client/src/nats/client/subscription.py | Subscription class supporting async iteration and context management |
| nats-client/src/nats/client/protocol/message.py | NATS protocol message parsing with support for MSG, HMSG, and control messages |
| nats-client/src/nats/client/protocol/command.py | Command encoding for NATS protocol operations (PUB, SUB, CONNECT, etc.) |
| nats-client/src/nats/client/message.py | Message data structures including Headers, Status, and Message classes |
| nats-client/tests/ | Comprehensive test suite covering client functionality, subscriptions, and protocol handling |
| nats-client/tools/bench.py | Benchmarking tool for performance testing |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
caspervonb
force-pushed
the
add-nats-server-package
branch
2 times, most recently
from
83d0f80 to
44fb982
Compare
caspervonb
force-pushed
the
add-nats-client-package
branch
from
781a9bf to
9a8cd71
Compare
|
Move this to be under |
philpennock
left a comment
philpennock
left a comment
There was a problem hiding this comment.
I don't see anything in the SSL context init to handle the server identity for TLS identity verification, when we learnt a reconnect address as an IP from the INFO line?
If we connect with a hostname originally, we validate that hostname in the cert, and if we reconnect to a learnt IP address, we validate that same original hostname as present in the new server cert. Did I just miss the handling of that?
caspervonb
force-pushed
the
add-nats-server-package
branch
3 times, most recently
from
accfd1c to
20963ae
Compare
caspervonb
changed the base branch from
main
to
migrate-project-to-uv-workspace
caspervonb
force-pushed
the
add-nats-client-package
branch
from
9a8cd71 to
0094dba
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 21 out of 22 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
.github/workflows/test.yml:1
- Git merge conflict markers are present in the CI configuration file. These need to be resolved before merging.
name: test
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 21 out of 22 changed files in this pull request and generated no new comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
caspervonb
changed the base branch from
migrate-project-to-uv-workspace
to
main
caspervonb
force-pushed
the
add-nats-client-package
branch
2 times, most recently
from
16d8621 to
7acfd1f
Compare
wallyqs
left a comment
wallyqs
left a comment
There was a problem hiding this comment.
Change the import to nats.experimental.aio.client while iterating on the implementation, then can work using that namespace in the other branches with the JetStream changes.
So Also would make all the currently pending PRs painful to merge and also the ones that aren't opened yet (e.g client auth, client tls, jetstream ordered consumer). Not sure what the benefit is of such a rename? |
caspervonb
force-pushed
the
add-nats-client-package
branch
from
7acfd1f to
2e13fe5
Compare
Signed-off-by: Casper Beyer <[email protected]>
Signed-off-by: Casper Beyer <[email protected]>
Signed-off-by: Casper Beyer <[email protected]>
Signed-off-by: Casper Beyer <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 56 out of 57 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Casper Beyer <[email protected]>
Signed-off-by: Casper Beyer <[email protected]>
Signed-off-by: Casper Beyer <[email protected]>
Signed-off-by: Casper Beyer <[email protected]>
Performance BenchmarksI did some further optimization and comparison. Test EnvironmentHardware:
Software:
Test Parameters:
Complete Results Table (Ordered by Subscriber Performance)
Key Findings1. Performance RankingsBy Subscriber Throughput:
2. Message Loss Analysisnats.aio Queue Mode - Critical Issue:
All other configurations: 0% message loss ✅ 3. Runtime ComparisonPyPy vs CPython (nats-client callback):
4. Implementation Comparisonnats-client vs nats.aio (callback mode):
Callback vs Queue (nats-client):
Performance ComparisonCallback Mode (Subscriber Throughput)
Queue Mode (Subscriber Throughput)
* nats.aio queue mode experiences severe message loss (47-96%) Summary
Benchmark Commandscd nats-client
# Individual tests
uv run python tools/bench.py --client client --messages 1000000 --size 128 [--callback]
uv run python tools/bench.py --client aio --messages 1000000 --size 128 [--callback] |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 56 out of 57 changed files in this pull request and generated 28 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| with open(current_file, "wb") as f: | ||
| # Wrap the file with progress tracker | ||
| progress_wrapper = ProgressFileWrapper(f, info.size, args.object) | ||
| result = await asyncio.wait_for( |
| super().__init__(f"{status}: {description}") | ||
|
|
||
| @classmethod | ||
| def from_status(cls, status: str, description: str, *, subject: str | None = None) -> StatusError: |
There was a problem hiding this comment.
Mixing implicit and explicit returns may indicate an error, as implicit returns always return None.
| return Pong("PONG") | ||
|
|
||
|
|
||
| async def parse(reader: Reader) -> Message | None: |
There was a problem hiding this comment.
Mixing implicit and explicit returns may indicate an error, as implicit returns always return None.
nats-client/tests/test_client.py
Outdated
| ) | ||
| async def test_reconnect_with_token(token): | ||
| """Test that client can reconnect to a token server after disconnection with all variants.""" | ||
| import asyncio |
nats-client/tests/test_client.py
Outdated
| ) | ||
| async def test_reconnect_with_user_pass(user, password): | ||
| """Test that client can reconnect to a user/pass server after disconnection with all variants.""" | ||
| import asyncio |
| for _ in range(message_count): | ||
| message2 = await sub2.next(timeout=3.0) | ||
| messages2.append(message2.data) | ||
| except asyncio.TimeoutError: |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
| task.cancel() | ||
| try: | ||
| await task | ||
| except asyncio.CancelledError: |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
| task.cancel() | ||
| try: | ||
| await task | ||
| except asyncio.CancelledError: |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
| task.cancel() | ||
| try: | ||
| await task | ||
| except asyncio.CancelledError: |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
| consumer.cancel() | ||
| try: | ||
| await consumer | ||
| except asyncio.CancelledError: |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
Signed-off-by: Casper Beyer <[email protected]>
* Add test for direct get returning no responders Signed-off-by: Casper Beyer <[email protected]>
Signed-off-by: Casper Beyer <[email protected]>
Signed-off-by: Casper Beyer <[email protected]>
Signed-off-by: Casper Beyer <[email protected]>
Signed-off-by: Casper Beyer <[email protected]>
Signed-off-by: Mark Jan van Kampen <[email protected]>
Signed-off-by: Casper Beyer <[email protected]>
* Merge branch 'ws/custom-ws-headers' into v2 * Fix formatting Signed-off-by: Casper Beyer <[email protected]>
Signed-off-by: Ksenia Vazhdaeva <[email protected]> Co-authored-by: Ksenia Vazhdaeva <[email protected]> Co-authored-by: Waldemar Quevedo <[email protected]>
Signed-off-by: Casper Beyer <[email protected]>
Signed-off-by: Casper Beyer <[email protected]>
Signed-off-by: Casper Beyer <[email protected]>
Signed-off-by: Casper Beyer <[email protected]>
Signed-off-by: Casper Beyer <[email protected]>
This adds a standalone nats.client package
Main differences in
nats.clientcompared tonats.aio