-
Notifications
You must be signed in to change notification settings - Fork 232
Add nats-server package #731
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
d2d090c to
9174ca1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a new nats-server Python package that provides programmatic control of NATS server instances for development and testing purposes. The package wraps the NATS server binary to enable easy server lifecycle management from Python code.
- Provides an asynchronous
Serverclass for managing NATS server processes - Implements a
run()function for starting servers with configurable options (port, JetStream, config files) - Includes comprehensive test coverage for various server configurations and edge cases
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| nats-server/src/nats/server/init.py | Core implementation with Server class and run() function for managing NATS server processes |
| nats-server/tests/test_server.py | Comprehensive test suite covering server startup, configuration, and error scenarios |
| nats-server/tests/conftest.py | Pytest fixtures for ensuring NATS server availability and port management |
| nats-server/tests/configs/*.conf | Test configuration files for various NATS server settings |
| nats-server/pyproject.toml | Package configuration with dependencies and build settings |
| nats-server/README.md | Package documentation with usage examples |
| .github/workflows/test.yml | CI workflow updates to include testing for the new package |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
83d0f80 to
44fb982
Compare
Jarema
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that this is missing cleanup of JetStream resources?
Referring to |
|
can't this all be under |
|
also should try to add cluster setup to smoke test features on R3s, publishing while there are leader elections, etc... |
Intent is to make it a workspace, where Top level package (nats) eventually becomes a set of feature-gated optional dependencies with re-exports. |
|
@caspervonb I would make some utility to easily create temp storage and cleanup method afterwards. After all, that will be used in every JetStream test. Whatever makes it convinient. |
|
I think you need to clarify the intent that this is for testing, so would prefer if you move to be under a |
wallyqs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Rollback from using src/ folder
- Use specific namespace for testing
- Add clustering example
Because of pytest fixtures, feels redundant.
Why tho? without it source files are littered in the package directory next to everything else, need to explicitly list each thing to be packaged in wheels, source dists etc.
How about we mark it explicitly as Changing the namespace means a lot of merge conflicts in the current merge queue.
No cluster support at the moment. But yes, we need to add that soon but not in the scope of this pull request imho. |
.github/workflows/test.yml
Outdated
| project: ["nats-server"] | ||
| steps: | ||
| - name: Checkout repository | ||
| uses: actions/checkout@v2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Current version is @v5; v2 used Node 12, and Node 16 is currently being deprecated by GH. I'm impressed this even works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bumped them.
.github/workflows/test.yml
Outdated
| uses: actions/checkout@v2 | ||
|
|
||
| - name: Set up Go | ||
| uses: actions/setup-go@v2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use @v6 for: actions/setup-go@v6; v2 was node12.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bumped them.
.github/workflows/test.yml
Outdated
| shell: bash | ||
|
|
||
| - name: Set up Python ${{ matrix.python-version }} | ||
| uses: actions/setup-python@v2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Python: @v6; v2 was node12.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bumped them.
5871d4f to
8c151c0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
@caspervonb, will there be support for the features from version 2.11 in this and related pulll requests? |
@Flosckow yes, new jetstream package will have everything. |
036825e to
ecb1a58
Compare
4d4529e to
accfd1c
Compare
Signed-off-by: Casper Beyer <casper@synadia.com>
accfd1c to
20963ae
Compare
This adds a nats.server package which provides utilities for spawning nats server processes.