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.

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

feat: support json protocol with grpc #437

Closed
wants to merge 1 commit into from

Conversation

xujianhai666
Copy link
Member

Closes #436

@codecov-io
Copy link

codecov-io commented Mar 21, 2020

Codecov Report

Merging #437 into master will decrease coverage by 0.68%.
The diff coverage is 40.9%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #437      +/-   ##
==========================================
- Coverage   67.41%   66.72%   -0.69%     
==========================================
  Files         174      175       +1     
  Lines        9261     9283      +22     
==========================================
- Hits         6243     6194      -49     
- Misses       2416     2484      +68     
- Partials      602      605       +3
Impacted Files Coverage Δ
protocol/grpc/codec.go 40.9% <40.9%> (ø)
filter/filter_impl/metrics_filter.go 86.36% <0%> (-13.64%) ⬇️
remoting/kubernetes/listener.go 56.07% <0%> (-9.35%) ⬇️
protocol/dubbo/listener.go 56.45% <0%> (-6.46%) ⬇️
protocol/dubbo/pool.go 74.88% <0%> (-6.4%) ⬇️
protocol/dubbo/client.go 64.24% <0%> (-4.85%) ⬇️
filter/filter_impl/hystrix_filter.go 68.64% <0%> (-3.39%) ⬇️
remoting/kubernetes/watch.go 78.3% <0%> (-2.84%) ⬇️
cluster/cluster_impl/failback_cluster_invoker.go 78.49% <0%> (-2.16%) ⬇️
remoting/kubernetes/client.go 58.36% <0%> (-0.66%) ⬇️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bde7db3...cb259e9. Read the comment docs.

@xujianhai666 xujianhai666 changed the title feat: support json protocol with grpc [WIP] feat: support json protocol with grpc Mar 21, 2020
@xujianhai666 xujianhai666 changed the title [WIP] feat: support json protocol with grpc feat: support json protocol with grpc Mar 28, 2020
- add client conf
- support json codec

Closes apache#436
}
}
clientConf = &defaultClientConfig
if err := clientConf.Validate(); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

It seems that the clientConfg always return nil.

}
}

func (c *ClientConfig) Validate() error {
Copy link
Member

Choose a reason for hiding this comment

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

Always return nil? Should we check the ContentType?

@flycash flycash assigned flycash and unassigned flycash Mar 31, 2020
conn, err := grpc.Dial(url.Location, grpc.WithInsecure(), grpc.WithBlock(),
grpc.WithUnaryInterceptor(
otgrpc.OpenTracingClientInterceptor(tracer, otgrpc.LogPayloads())))
dailOpts := make([]grpc.DialOption, 0)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
dailOpts := make([]grpc.DialOption, 0)
dailOpts := make([]grpc.DialOption, 0,n)

specify capacity as above

* See the License for the specific language governing permissions and
* limitations under the License.
*/
package grpc
Copy link
Member

Choose a reason for hiding this comment

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

add blank line

* See the License for the specific language governing permissions and
* limitations under the License.
*/
package grpc
Copy link
Member

Choose a reason for hiding this comment

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

add blank line

@zouyx
Copy link
Member

zouyx commented Apr 2, 2020

why merge to master?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: support Json protocol for grpc
5 participants