Kuo-Chung Hsu

Results 40 comments of Kuo-Chung Hsu

The basic flow is verified by unit tests. While I worked on testing corner cases, could I have some initial feedback? cc @zuercher @rgs1 @fishcakez @PeterL328

Some code decisions: (a) `UpstreamRequest` is not reusable since HC (Health Checker) creates `ClientConnection` directly but `UpstreamRequest` uses `ConnectionPool`. Therefore we implement a simple thrift client. (b) Extract `ThriftHealthChecker::Client` for...

@markdroth heads up that the reason we don't use `not_in` to exclude AUTO and TWITTER is a bug for foreign enum which triggers the compile error ``` bazel-out/k8-dbg/bin/external/envoy_api/envoy/extensions/health_checkers/thrift/v3/thrift.pb.validate.cc:62:17: error: unknown...

Code coverage for source/extensions/health_checkers/thrift is lower than limit of 96.6 (89.3) more boundary cases will help

With some discussion with @fishcakez, we should use fixed sequence id 0.

@markdroth the implantation review is converging. would you have a chance to review api part? thanks.

@fishcakez noticed that we don't specify transport and protocol in thrift.proto to be required since protoc-gen doesn't support `(validate.rules).enum.required`. Fortunately we throw for AUTO_PROTOCOL and AUTO_TRANSPORT, which is 0 and...

Thanks Adi. Leave a note that we've verified thrift HC in our environment.