-
Notifications
You must be signed in to change notification settings - Fork 191
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
feat(kv): implement KeysWithFilters
method for key filtering in KV bucket
#602
Conversation
…cket - Added KeysWithFilters method to KeyValue store to allow retrieving keys that match specific filters. - The method returns a list of keys from the KV bucket, applying the provided filters for pattern matching. - Checks consumer info to ensure filters are properly supported by the NATS server, with warnings for server versions < 2.10. Resolves: [nats-io#575](nats-io#575)
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.
Thanks for the contribution! I left some comments, before including we need to add a test for the feature.
- replace print with logger - added params to existing method keys() - add test file - update example - raise error
- replace print with logger - added params to existing method keys() - add test file - update example - raise error
thanks @wallyqs for the quick review, i have addressed the comments, please check once. |
try: | ||
consumer_info = await watcher._sub.consumer_info() | ||
if consumer_info and filters: | ||
# If NATS server < 2.10, filters might be ignored. |
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.
there is a connected_server_version
which can be used to check: https://github.com/nats-io/nats.py/blob/main/nats/aio/client.py#L1210C9-L1210C33
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.
this can't be invoked without an instance of nc
I suppose?
I checked the usage of connected_server_version
and this is how its been used
server_version = nc.connected_server_version
I believe creating an instance of nc won't be the best idea.
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.
think it is part of the state already here, maybe via self._js._nc
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.
no,
I have tried that already 😄
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 see, thanks for checking
- remove unnecessary logging line
KeysWithFilters
method for key filtering in KV bucket
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.
LGTM, thanks!
Resolves: #575