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.

Sign up for GitHub

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

Jump to bottom

ISPN-14640 RPUSH and RPUSHX commands #10874

Merged
merged 1 commit into from
May 5, 2023
Merged

ISPN-14640 RPUSH and RPUSHX commands #10874

merged 1 commit into from
May 5, 2023

Conversation

karesti
Copy link
Contributor

@karesti karesti commented Apr 27, 2023 •
edited

@karesti karesti marked this pull request as draft April 27, 2023 16:41
@karesti karesti changed the title List commands ISPN-14640 RPUSH command Apr 27, 2023
@karesti karesti requested review from wburns and jabolina April 27, 2023 16:41
@karesti
Copy link
Contributor Author

karesti commented Apr 27, 2023

@wburns @jabolina I think I need a master class on CompletionStages class
I left a TODO on the multimap that brings a io.lettuce.core.RedisCommandExecutionException: ERR org.infinispan.commons.CacheException: class [B cannot be cast to class org.infinispan.multimap.impl.Bucket ([B is in module java.base of loader 'bootstrap'; org.infinispan.multimap.impl.Bucket is in unnamed module of loader 'app')

We need to deal that if a key exists, and it's not a multimap list, then we need to deal the WRONGTYPE Operation against a key holding the wrong kind of value

@karesti karesti marked this pull request as ready for review April 28, 2023 09:35
@karesti
Copy link
Contributor Author

karesti commented Apr 28, 2023

@jabolina @wburns I implemented the commands

@karesti karesti changed the title ISPN-14640 RPUSH command ISPN-14640 RPUSH and RPUSHX commands Apr 28, 2023
@karesti karesti force-pushed the list-commands branch 2 times, most recently from 7dd0e09 to 7ccd25f Compare May 2, 2023 07:52
jabolina
jabolina reviewed May 2, 2023
View reviewed changes
Copy link
Member

@jabolina jabolina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one thing, but looking good.

server/resp/src/main/java/org/infinispan/server/resp/commands/list/RPUSH.java Outdated Show resolved Hide resolved
server/resp/src/main/java/org/infinispan/server/resp/commands/list/RPUSHX.java Outdated Show resolved Hide resolved
server/resp/src/main/java/org/infinispan/server/resp/commands/list/RPUSHX.java Outdated Show resolved Hide resolved
@karesti
Copy link
Contributor Author

karesti commented May 2, 2023

@jabolina I reported the changes

@karesti karesti removed the request for review from wburns May 2, 2023 20:00
wburns
wburns reviewed May 3, 2023
View reviewed changes
Copy link
Member

@wburns wburns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some minor stuff

server/resp/src/main/java/org/infinispan/server/resp/commands/list/RPUSHX.java Outdated Show resolved Hide resolved
server/resp/src/main/java/org/infinispan/server/resp/commands/list/RPUSH.java Show resolved Hide resolved
@karesti karesti force-pushed the list-commands branch 2 times, most recently from 3f01c9b to fcec859 Compare May 4, 2023 06:31
@karesti
Copy link
Contributor Author

karesti commented May 4, 2023

@wburns @jabolina I reported the changes

@jabolina
Copy link
Member

jabolina commented May 4, 2023

@karesti Needs rebase.

jabolina
jabolina approved these changes May 4, 2023
View reviewed changes
Copy link
Member

@jabolina jabolina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@jabolina
Copy link
Member

jabolina commented May 4, 2023

@karesti CI complaining about some unused imports during checkstyle. Just that and we can merge this one.

@jabolina jabolina merged commit 0d15f8e into infinispan:main May 5, 2023
3 of 4 checks passed
@jabolina
Copy link
Member

jabolina commented May 5, 2023

Integrated into main, thanks @karesti

@karesti karesti deleted the list-commands branch May 6, 2023 07:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wburns wburns wburns approved these changes

@jabolina jabolina jabolina approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@karesti @jabolina @wburns