-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
[redis_proxy] Add support for SELECT and KEYS #37706
Conversation
Hi @duanhongyi, welcome and thank you for your contribution. We will try to review your Pull Request as quickly as possible. In the meantime, please take a look at the contribution guidelines if you have not done so already. |
8585977
to
2d6baa7
Compare
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.
Assigning @weisisea @mattklein123 as codeowners
/assign @weisisea @mattklein123
ef12e37
to
d5bd91f
Compare
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 at a high level. Can you fix the typo comments and then push to see if we can get CI green?
/wait
source/extensions/filters/network/redis_proxy/conn_pool_impl.cc
Outdated
Show resolved
Hide resolved
9edd6d8
to
234aa23
Compare
I rebased the last commit, mainly to solve the compatibility issues of the Java client. |
8af3e65
to
488fcda
Compare
There is still one that hasn't turned green, I will add some test cases later.
|
85b3e73
to
7fb8c98
Compare
Signed-off-by: duanhongyi <duanhongyi@doopai.com>
Signed-off-by: duanhongyi <duanhongyi@doopai.com>
Signed-off-by: duanhongyi <duanhongyi@doopai.com>
Okay, all the checks have been passed. |
This RP mainly improves the compatibility of redis proxy, improvement as follows: