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

protocol: add the framework of x protocol, and cmdline arguments. #3618

Merged
merged 12 commits into from Aug 22, 2017

Conversation

Projects
None yet
5 participants
@hicqu
Contributor

hicqu commented Jul 4, 2017

This is a basic framework for x protocol server side.

X protocol is a new C/S protocol for MySQL from 5.7. It offers document-store access. And It's defined by proto instead of text specifies, so it's more easily to be implemented. MySQL Shell and Mysql Connector 7.0.2 higher have implemented it already.

@hicqu hicqu requested review from shenli, tiancaiamao and zimulala Jul 4, 2017

@hicqu

This comment has been minimized.

Show comment
Hide comment
@hicqu

hicqu Jul 4, 2017

Contributor
Contributor

hicqu commented Jul 4, 2017

@jackysp jackysp added the status/LGT1 label Jul 6, 2017

@zimulala

This comment has been minimized.

Show comment
Hide comment
@zimulala

zimulala Jul 8, 2017

Member

I don't know what x protocol server is used for. Can you add some explanation?

Member

zimulala commented Jul 8, 2017

I don't know what x protocol server is used for. Can you add some explanation?

@zimulala

This comment has been minimized.

Show comment
Hide comment
@zimulala

zimulala Jul 10, 2017

Member

Please add some tests.

Member

zimulala commented Jul 10, 2017

Please add some tests.

@hicqu hicqu closed this Jul 14, 2017

@hicqu hicqu reopened this Aug 8, 2017

hicqu added some commits Aug 8, 2017

@tiancaiamao

Almost the same as server package, could you make better abstraction and don't repeat the code?

Show outdated Hide outdated tidb-server/main.go
@@ -0,0 +1,31 @@
// Copyright 2017 PingCAP, Inc.

This comment has been minimized.

@tiancaiamao

tiancaiamao Aug 11, 2017

Contributor

You make it a separate package, but not test for it, our coverage down continuous.

@tiancaiamao

tiancaiamao Aug 11, 2017

Contributor

You make it a separate package, but not test for it, our coverage down continuous.

This comment has been minimized.

@hicqu

hicqu Aug 21, 2017

Contributor

It's just a networking level stub. We can only add test cases after we implement them in business logic level, just like we do in server/{sever.go,conn.go}.

@hicqu

hicqu Aug 21, 2017

Contributor

It's just a networking level stub. We can only add test cases after we implement them in business logic level, just like we do in server/{sever.go,conn.go}.

@jackysp

This comment has been minimized.

Show comment
Hide comment
@jackysp

jackysp Aug 21, 2017

Member

Please resolve the conflicts.

Member

jackysp commented Aug 21, 2017

Please resolve the conflicts.

Show outdated Hide outdated tidb-server/main.go
@wentaoxu

LGTM

@jackysp jackysp merged commit 38908ae into master Aug 22, 2017

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

@jackysp jackysp deleted the qupeng/x-protocol branch Aug 22, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment