Skip to content
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

Consider expanding env vars while using request options #291

Open
kedarmhaswade opened this issue Nov 14, 2019 · 4 comments
Open

Consider expanding env vars while using request options #291

kedarmhaswade opened this issue Nov 14, 2019 · 4 comments

Comments

@kedarmhaswade
Copy link

kedarmhaswade commented Nov 14, 2019

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"
@kedarmhaswade kedarmhaswade changed the title Consider expanding env vars while using request options? Consider expanding env vars while using request options Nov 14, 2019
@prashantv
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

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.

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

No branches or pull requests

3 participants