Skip to content
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

Add overloads to StreamOperations accepting XAddOptions #2915

Closed
sanwzzz opened this issue May 24, 2024 · 7 comments
Closed

Add overloads to StreamOperations accepting XAddOptions #2915

sanwzzz opened this issue May 24, 2024 · 7 comments
Labels
type: enhancement A general enhancement

Comments

@sanwzzz
Copy link

sanwzzz commented May 24, 2024

HI

@sanwzzz sanwzzz closed this as completed May 24, 2024
@sanwzzz
Copy link
Author

sanwzzz commented May 24, 2024

Hi,

I am currently using redisTemplate.opsForStream() to operate on Redis streams. When using the add method, I want to support more parameters such as MAXID and MINID. However, it is not supported at the moment. I noticed that RedisStreamCommands already provides XAddOptions, but currently (in version 3.3.X), there is no way to set these options, and only the default None is called.

Is it possible to enhance the DefaultStreamOperations.add method to support these parameters?

Example

@sanwzzz
Copy link
Author

sanwzzz commented May 24, 2024

default RecordId add(Record<K, ?> record, XAddOptions ops) {
    return add(record, null)
};
RecordId add(Record<K, ?> record, XAddOptions ops);

@sanwzzz sanwzzz reopened this May 24, 2024
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label May 24, 2024
@mp911de mp911de changed the title redisTemplate.opsForStream().add Add support XAddOptions parmams redisTemplate.opsForStream().add Add support XAddOptions Jun 24, 2024
@mp911de mp911de added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Jun 24, 2024
@mp911de
Copy link
Member

mp911de commented Jun 24, 2024

Not exactly sure why you've posted details about DefaultStreamOperations. In any case, adding options for the XADD operations are held by XAddOptions. minId and maxlen are already supported, so not sure what you're asking for. Please elaborate.

@mp911de mp911de added the status: waiting-for-triage An issue we've not yet triaged label Jun 24, 2024
@sanwzzz
Copy link
Author

sanwzzz commented Jun 25, 2024

Not exactly sure why you've posted details about DefaultStreamOperations. In any case, adding options for the XADD operations are held by XAddOptions. minId and maxlen are already supported, so not sure what you're asking for. Please elaborate.

I want to set the XAddOptions parameter in redisTemplate.opsForStream().add(...) , but there is no method to set the XAddOptions parameter.

Is it possible to add the following API
RecordId add(Record<K, ? > record, XAddOptions ops);

@mp911de
Copy link
Member

mp911de commented Jun 27, 2024

Ah, I got it. All the screenshots are pretty distracting and contribute a lot of noise to this ticket. I went ahead and removed these.

It is fine to add overloads accepting XAddOptions in StreamOperations and ReactiveStreamOperations.

@mp911de mp911de changed the title redisTemplate.opsForStream().add Add support XAddOptions Add overloads to StreamOperations accepting XAddOptions Jun 27, 2024
@mp911de mp911de removed the status: waiting-for-triage An issue we've not yet triaged label Jun 27, 2024
@jinkshower
Copy link
Contributor

Hello, @mp911de
May I take this issue?

I've looked through issue and code. I am planning to add interface method
RecordId add(Record<K, ?> record, XAddOptions options)
and override this method in DefaultStreamOperations.
and also same with ReactiveStreamOperations

Do you expect the client code should be like this?

XAddOptions options = XAddOptions.maxlen(1000);
redisTemplate.opsForStream().add("stream", Map.of("key", "value"), options);

if this is the way you implied, i'm happy to make a pr with a few tests to support this change.

jinkshower added a commit to jinkshower/spring-data-redis that referenced this issue Jul 7, 2024
jinkshower added a commit to jinkshower/spring-data-redis that referenced this issue Jul 8, 2024
@sanwzzz
Copy link
Author

sanwzzz commented Jul 9, 2024

Is it possible set global default options?
Clip_2024-07-09_15-07-35

jinkshower added a commit to jinkshower/spring-data-redis that referenced this issue Jul 10, 2024
jinkshower added a commit to jinkshower/spring-data-redis that referenced this issue Jul 18, 2024
jinkshower added a commit to jinkshower/spring-data-redis that referenced this issue Jul 24, 2024
jinkshower added a commit to jinkshower/spring-data-redis that referenced this issue Jul 29, 2024
@christophstrobl christophstrobl added this to the 3.4 M1 (2024.1.0) milestone Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants