Support MINID in XADD #318
Merged
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
|
Hi, thanks for the PR! Looks good on first glance, but I'll have a proper look another day. Would it be possible to add some tests in the ./integration/ dir? If not also no problem. |
|
Hi @alicebob - definitely I can add some integration tests. I'll get started on those soon. |
|
On Fri, Mar 24, 2023 at 10:23:55AM -0700, Nathan Cormier wrote:
Hi @alicebob - definitely I can add some integration tests. I'll get started on those soon.
That would be great. Let me know if you're stuck.
Also it looks like the Go 1.14 tests fail because `UnixMilli` didn't exist on that version, so I can refactor that out and calculate milliseconds.
I remember another PR did something similar recently. But I frankly also don't
mind upping the lowest supported version to Go 1.17, which has UnixMilli
(that's than 4 releases).
|
|
Hi @alicebob I added some integration tests, let me know how it looks! |
|
thanks for the fix! Code looks great, if you can squash it to a single commit I'll merge it. |
According to https://redis.io/commands/xadd XADD should support MINID (in addition to MAXLEN) as another means of trimming old entries from the stream. First added in Redis 6.2.0 https://raw.githubusercontent.com/redis/redis/6.2/00-RELEASENOTES
|
You're welcome @alicebob ! Just squashed the commits. |
|
great, thanks! I'll review the other open PR soon and then make a new version. |
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.
According to https://redis.io/commands/xadd
XADD should support MINID (in addition to MAXLEN) as another means of trimming old entries from the stream.
First added in Redis 6.2.0 https://raw.githubusercontent.com/redis/redis/6.2/00-RELEASENOTES