Move common parts of redis connection interface to common#6177
Merged
mattklein123 merged 9 commits intoenvoyproxy:masterfrom Mar 8, 2019
Merged
Move common parts of redis connection interface to common#6177mattklein123 merged 9 commits intoenvoyproxy:masterfrom
mattklein123 merged 9 commits intoenvoyproxy:masterfrom
Conversation
Signed-off-by: Nicolas Flacco <nflacco@lyft.com>
Signed-off-by: Nicolas Flacco <nflacco@lyft.com>
Contributor
Author
|
oops, I'll fix those errors |
Member
|
@HenryYYang for first pass. |
Contributor
Author
|
As it turns out, the formatter rearranges imports, and this breaks the build! Hence my testing working locally, as I didn't test post-formatto. |
Signed-off-by: Nicolas Flacco <nflacco@lyft.com>
Signed-off-by: Nicolas Flacco <nflacco@lyft.com>
6142d48 to
2b2f153
Compare
Signed-off-by: Nicolas Flacco <nflacco@lyft.com>
f51c2ec to
78f6007
Compare
HenryYYang
reviewed
Mar 6, 2019
Signed-off-by: Nicolas Flacco <nflacco@lyft.com>
Signed-off-by: Nicolas Flacco <nflacco@lyft.com>
Signed-off-by: Nicolas Flacco <nflacco@lyft.com>
Signed-off-by: Nicolas Flacco <nflacco@lyft.com>
HenryYYang
reviewed
Mar 8, 2019
| namespace Redis { | ||
| namespace Client { | ||
|
|
||
| inline envoy::config::filter::network::redis_proxy::v2::RedisProxy::ConnPoolSettings |
Contributor
There was a problem hiding this comment.
this was not inline before, right?
Member
There was a problem hiding this comment.
This is fine, or you can move the impl to a CC file in a follow up.
Contributor
Author
There was a problem hiding this comment.
It was not- I inlined it as it was short (per the style guide this ok apparently), but it's an easy move if you guys prefer.
Contributor
|
LGTM |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
This is part of the effort to build a proxy for Redis Cluster (#5697).
We want to make some of the connection interface common so we can share it with the cluster proxy. Specifically, we take
redis_proxy/conn_pool.hand move much of it besidesInstancetocommon/redis/client.h, and adjust the rest of the project to work with this. The formatting tool did some spacing stuff as well.Following this, we will want to make an upstream command abstraction (for commands sent by Envoy upstream and that come back and terminate at Envoy) such as the health checks (exist, ping) and cluster (cluster slots).