yarpc / yab

Call and benchmark YARPC services from the command line.
MIT License
84 stars 33 forks source link

feature: -t/--thrift flag traverses directories #133

Open APTy opened 7 years ago

APTy commented 7 years ago

Feature request

If -t/--thrift is passed a directory, yab will traverse it to find an appropriate *.thrift definition file for the service being called.

Motivation

The primary motivator for this change is simplicity for end-users.

In ecosystems where users make adhoc yab calls across numerous microservices, it can be cumbersome to constantly specify a *.thrift file with the -t/--thrift flag. Users will be able to specify a root IDL directory (e.g. /usr/share/my-idls/), and yab will find the right definition file for the service being called.

Further, the -t/--thrift setting could be predefined in defaults.ini to reduce the need for supplying the flag altogether.

Discussion

Namespace collisions of service names could make it difficult to find the thrift file for the intended service being invoked. This shouldn't be a problem for most usage, but it's not immediately clear how to handle those edge cases.

Additionally, there is some non-trivial overhead to traversing a large set of thrift definition files. For a tool like yab, this is possibly undesirable, but users always have the option to explicitly list a file using the -t/--thrift flag if speed becomes a concern.

prashantv commented 7 years ago

@APTy I've thought about something similar myself and have a branch in progress that adds:

https://github.com/yarpc/yab/tree/dir_scan

If there's any collisions, it would probably just fail and ask the user to specify the full path, but I do think that most thrift methods wouldn't clash.

There would be a performance impact, but like you said it can be avoided by specifying the full file. In future we could build an index to make it faster too.

This branch was started a few months ago, I just haven't had the time to complete it, but it's definitely on my radar.