-
Notifications
You must be signed in to change notification settings - Fork 151
Implement tchannel crossdock tests #13
Conversation
crossdock/server/serializer.py
Outdated
|
|
||
| def downstream_to_json(d): | ||
| s = {} | ||
| def join_trace_request_to_json(d, s): |
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.
please use readable variable names
crossdock/server/server.py
Outdated
| @tchannel.thrift.register(service.TracedService, method='startTrace') | ||
| @tornado.gen.coroutine | ||
| def start_trace(request): | ||
| raise UnimplementedEndpointException("tchannel startTrace endpoint not implemented") |
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.
just don't add this endpoint to begin with, tchannel will throw some exception automatically. And we won't need UnimplementedEndpointException
|
Q: somewhat suspicious about coverage numbers not changing at all. Are the crossdock files being tested? |
crossdock/server/server.py
Outdated
| @tchannel.thrift.register(service.TracedService, method='joinTrace') | ||
| @tornado.gen.coroutine | ||
| def join_trace(request): | ||
| jtr = request.body.request if request.body.request else None |
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.
nit: request.body.request or None
crossdock/server/serializer.py
Outdated
| a == 'type_spec'] | ||
|
|
||
|
|
||
| def obj_to_json(obj): |
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.
which obj is this?
crossdock/docker-compose.yml
Outdated
| go: | ||
| image: jaegertracing/xdock-go | ||
| # TODO Use the master branch once this branch is merged | ||
| image: jaegertracing/xdock-go:upgrade-crossdock-tests-for-native-opentracing-in-tchannel |
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 Go repo has been upgraded, you can use the latest image now (rm the tag ^^^)
|
LGTM Pull the latest Go img, and |
No description provided.