yarpc / yab

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

Consider expanding env vars while using request options #291

Open kedarmhaswade opened 4 years ago

kedarmhaswade commented 4 years ago

Although we have our own name: ${name:value} mechanism implemented in yab, can we consider the values for the variables as a merge of env vars and those that are specified on the command line (command line taking the precedence)? I miss the expansion of ${HOME} to the path of my home folder.

Example yab

#!/usr/bin/env yab

service: hello
timeout: 2h
peer: localhost:6707
method: Hello::greet
#thrift: /Users/joe/gocode/src/idl/hello.thrift
thrift: ${HOME}/gocode/src/idl/hello.thrift
request:
    actorID: ${actorID:joe@blo.com}

Somewhat unexpected error

➜  yabs ./is-super-admin.yab -A "actorID: dave@blo.com"
Failed while parsing input: cannot find Thrift file: "/Users/joe/yabs/${HOME}/gocode/src/idl/hello.thrift"
prashantv commented 4 years ago

I can see the value in using environment variables, not sure if we should do it by default, or have an opt-in "fallback" to the environment variables. Do you have any thoughts on this @kriskowal?

Separately, for what you're doing, we suggest putting the .yab files close to the .thrift files, maybe in a sub-directory or close by, and use relative paths like ../hello.thrift. That way the yab files are distributed along with your .thrift files, while also being more portable to different environments/users.

kriskowal commented 4 years ago

For this specific case, using a path relative to the yab file is the proper solution. That’s a bit of a bear in the monorepo, where the IDL and scripts are likely in parallel universes, but as @prashantv mentions, the intent is for you to put your IDL and yab scripts in the same directory, since it’s handy to distribute them to client repositories.

Using an environment variable in this case would likely be more brittle than the relative path, though $GOPATH is less brittle than $HOME/go-code.

I think we should wait for a more substantial feature before we entertain interpolating both -A arguments and environment variables (or unifying their namespace).

kedarmhaswade commented 4 years ago

I agree that this is not such a big deal. We may wait for more people to run into this before making any change. It's just that on the surface, $HOME not expanding to the path to my home folder felt like a POLA violation on a Unix computer: when a file has $HOME or ${HOME}, it is more likely that it is a deliberate act rather than a mistake.

I also agree that keeping yab files closer to thrift files is a good suggestion: using relative paths just becomes natural that way. However, yab should handle the specification of a file path in a (more-or-less) standard manner: If it starts with anything other than a forward slash then it is a relative path, absolute otherwise.

kriskowal commented 4 years ago

However, yab should handle the specification of a file path in a (more-or-less) standard manner: If it starts with anything other than a forward slash then it is a relative path, absolute otherwise.

Agreed. I believe this is the case, but if it’s a bug if you find otherwise.