-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Feature grpc version echo api #321
Feature grpc version echo api #321
Conversation
Started the GRPC APIs with Version API
Added the test cases for GRPC apis
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JayKaku I suggest you rewrite the Git history and have a commit for the api/http rename, one for the grpc and another with echo/panic/version.
server := health.NewServer() | ||
|
||
// Register grpc apis for refection | ||
echo.RegisterEchoServiceServer(srv, &echoServer{}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The other servers ake a config and logger args, should't this do the same?
} | ||
|
||
func (s *echoServer) Echo (ctx context.Context, message *echo.Message) (*echo.Message, error){ | ||
log.Printf("Received message body from client: %s", message.Body) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to use the server logger instead of default one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright
tash
Outdated
@@ -0,0 +1,13 @@ | |||
[1mdiff --git a/cmd/podinfo/main.go b/cmd/podinfo/main.go[m |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this please
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure
@stefanprodan we are planning to raise another PR because the git history is entangled |
Superseded by #322 |
#318 @stefanprodan : Added
version
andecho
grpc apis and restructured folders and changedpackage api
topackage http