Skip to content

Conversation

@PeterRK
Copy link

@PeterRK PeterRK commented Sep 11, 2018

放开Thrift的多参数支持

@PeterRK PeterRK closed this Sep 11, 2018
@PeterRK PeterRK reopened this Sep 11, 2018
@PeterRK PeterRK changed the title support thrift rpc with multipule arguments support thrift rpc with multipule arguments & fix read_invariant_cpu_frequency Sep 13, 2018
@PeterRK PeterRK changed the title support thrift rpc with multipule arguments & fix read_invariant_cpu_frequency support thrift rpc with multipule arguments Sep 13, 2018
xfer += oprot.writeFieldBegin("request", ::apache::thrift::protocol::T_STRUCT,
THRIFT_REQUEST_FID);
// xfer += oprot.writeFieldBegin("request", ::apache::thrift::protocol::T_STRUCT,
// THRIFT_REQUEST_FID);
Copy link
Contributor

Choose a reason for hiding this comment

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

这个行为应该作为controller的一个参数,参数不同而行为不同,而不是直接注释掉,毕竟很多情况下还是只有一个request,要传递xxx_args是有些麻烦的。

Choose a reason for hiding this comment

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

感觉按照官方的thrift_gen的版本,xx_args是标准的实现,单参数是『恰好』可以work而已

Copy link
Contributor

Choose a reason for hiding this comment

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

确实是这样。不过这里主要是从使用体验考虑,一个参数的情况还是很多的,使用xxx_args未免很繁琐。我也比较倾向于只用一个参数。这个行为不一定要用户指定,由于CallMethod是基于模版的,所以这儿可以使用一个特殊的模板类来判定request类型名是不是形如"Service_Method_args",有误判可能性,但概率很低(相信正常心智的程序员都不会把类命名为Service_Method_args这种形式吧)。这个判定可以只对每个类型只做一次,而不是每次CallMethod前都做。

void ThriftStub::CallMethod(const char* method_name,
Controller* cntl,
const REQUEST* raw_request,
::google::protobuf::Closure* done) {
Copy link
Contributor

Choose a reason for hiding this comment

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

这些代码不需要新启函数,只需要针对raw_response为空的情况特殊处理就行了?

Choose a reason for hiding this comment

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

这个还有下文吗?能支持thrift多参数的情况嘛

@stale
Copy link

stale bot commented Nov 17, 2018

This issue has been automatically marked as stale because it's inactive for a long time. It will be closed if no further activity occurs, reopen if you have further ideas. Thank you for your contributions! 由于很久没有活跃,此Issue已被自动标记为过期。之后几天仍无变化的话将会被关闭,若你有新想法则可重新打开。感谢你的贡献!

@stale stale bot added the wontfix Not belonging to other labels label Nov 17, 2018
@stale stale bot closed this Nov 22, 2018
@threadfly
Copy link

能支持了吗

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

Labels

wontfix Not belonging to other labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants