Refactor Redis commands and connection pool#6149
Conversation
fe379bb to
83197a4
Compare
redis_cluster_proxy work. Signed-off-by: Nicolas Flacco <nflacco@lyft.com>
83197a4 to
5d9e015
Compare
|
@FAYiEKcbD0XFqF2QK2E4viAHg8rMm2VbjYKdjTg your actual error is: |
Signed-off-by: Nicolas Flacco <nflacco@lyft.com>
e10f89f to
b7b78cb
Compare
Signed-off-by: Nicolas Flacco <nflacco@lyft.com>
|
+1 |
|
@mattklein123 don't know how I missed that in the bottom of the output there. In any case, it is fixed. |
mattklein123
left a comment
There was a problem hiding this comment.
Thanks looks good, just a few questions.
/wait
| @@ -0,0 +1,40 @@ | |||
| licenses(["notice"]) # Apache 2 | |||
|
|
|||
| # Redis proxy L4 network filter. Implements consistent hashing and observability for large redis | |||
There was a problem hiding this comment.
Can you delete this comment as I don't think applies here now that it's common code?
There was a problem hiding this comment.
done
| load( | ||
| "//test/extensions:extensions_build_system.bzl", | ||
| "envoy_cc_test", | ||
| "envoy_extension_cc_test_binary", |
There was a problem hiding this comment.
This is not used, I would remove this entire load() and just import the normal envoy_cc_test from the normal build size bzl file.
There was a problem hiding this comment.
done
| EXPECT_EQ(0UL, buffer_.length()); | ||
| } | ||
|
|
||
| TEST_F(RedisEncoderDecoderImplTest, BulkString) { |
There was a problem hiding this comment.
Was this test missing? Why was this added?
There was a problem hiding this comment.
This test is new- since a bunch of the other types are there, I figured I'd add it. In theory the array tests capture it; I think one could probably remove some of the other types and do them in the array tests too, but it reads nicer. I can remove if you prefer.
There was a problem hiding this comment.
Nope thanks for adding, just curious.
Signed-off-by: Nicolas Flacco <nflacco@lyft.com>
Signed-off-by: Nicolas Flacco <nflacco@lyft.com> Signed-off-by: Fred Douglas <fredlas@google.com>
Simpler version of #6055. Will amend description/close other PR if this one passes coverage test without a strange bison/flex error.