Node: Add XRANGE command #2069
Merged
Node: Add XRANGE command #2069
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Jonathan Louie <[email protected]>
Signed-off-by: Jonathan Louie <[email protected]>
Signed-off-by: Jonathan Louie <[email protected]>
Signed-off-by: Jonathan Louie <[email protected]>
Signed-off-by: Jonathan Louie <[email protected]>
Signed-off-by: Jonathan Louie <[email protected]>
Signed-off-by: Jonathan Louie <[email protected]>
Signed-off-by: Jonathan Louie <[email protected]>
Signed-off-by: Jonathan Louie <[email protected]>
acarbonetto
reviewed
Aug 1, 2024
node/src/BaseClient.ts
Outdated
| * ```typescript | ||
| * await client.xadd("mystream", [["field1", "value1"]], {id: "0-1"}); | ||
| * await client.xadd("mystream", [["field2", "value2"], ["field2", "value3"]], {id: "0-2"}); | ||
| * const result = await client.xrange("mystream", "-", "+"); |
There was a problem hiding this comment.
Suggested change
| * const result = await client.xrange("mystream", "-", "+"); | |
| * console.log(await client.xrange("mystream", "-", "+")); |
node/src/Transaction.ts
Outdated
| * See https://valkey.io/commands/xrange for more details. | ||
| * | ||
| * @param key - The key of the stream. | ||
| * @param start - The starting stream ID bound for the range. |
There was a problem hiding this comment.
mention what id is for the range bound?
tjzhang-BQ
approved these changes
Aug 1, 2024
Signed-off-by: Jonathan Louie <[email protected]>
Signed-off-by: Jonathan Louie <[email protected]>
Signed-off-by: Jonathan Louie <[email protected]>
Signed-off-by: Jonathan Louie <[email protected]>
Signed-off-by: Jonathan Louie <[email protected]>
Signed-off-by: Jonathan Louie <[email protected]>
Signed-off-by: Jonathan Louie <[email protected]>
Signed-off-by: Jonathan Louie <[email protected]>
Signed-off-by: Jonathan Louie <[email protected]>
You need to reflect this in docs and in tests |
Well this is getting kind of frustrating. I'm following the Python tests and there's two things that stand out to me. They use numbers for timestamps for one thing, which apparently isn't how they're specified according to your previous comment. Secondly, this check for the Redis version isn't actually in the Python xrange tests at all. |
…ion to xrange tests Signed-off-by: Jonathan Louie <[email protected]>
Yury-Fridlyand
approved these changes
Aug 13, 2024
Signed-off-by: Jonathan Louie <[email protected]>
Yury-Fridlyand
approved these changes
Aug 14, 2024
node/src/BaseClient.ts
Outdated
| * ```typescript | ||
| * await client.xadd("mystream", [["field1", "value1"]], {id: "0-1"}); | ||
| * await client.xadd("mystream", [["field2", "value2"], ["field2", "value3"]], {id: "0-2"}); | ||
| * console.log(await client.xrange("mystream", "-", "+")); |
There was a problem hiding this comment.
Please update the example to use the enum properly.
node/src/BaseClient.ts
Outdated
| * - Use `InfBoundary.PositiveInfinity` to end with the maximum available ID. | ||
| * @param count - An optional argument specifying the maximum count of stream entries to return. | ||
| * If `count` is not provided, all stream entries in the range will be returned. | ||
| * @returns A map of stream entry ids, to an array of entries. |
There was a problem hiding this comment.
Tell when it return null.
There was a problem hiding this comment.
https://github.com/valkey-io/valkey-glide/pull/1920/files I guess this should've added documentation for the Java client about the null case when it was fixed? It certainly doesn't seem to be there right now on main.
acarbonetto
approved these changes
Aug 14, 2024
| */ | ||
| export type ScoreBoundary<T> = | ||
| export type Boundary<T> = |
Signed-off-by: Jonathan Louie <[email protected]>
Yury-Fridlyand
approved these changes
Aug 14, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://valkey.io/commands/xrange/