yext / edward

A tool for managing local microservice instances
http://engblog.yext.com/edward/
MIT License
171 stars 32 forks source link

Expand ENV variables in watches and base from service.path. #133

Open bradobro opened 7 years ago

bradobro commented 7 years ago

Description

Given I have to orchestrate several development services, some of which cannot live under $GOPATH. When I tell a service to watch the non-absolute directory, lib Then I expect those directories to be treated as relative to that service's path. Instead I see the error, Could not enable auto-restart: lstat lib: no such file or directory.

My anticipated workaround doesn't help:

When I add an environment variable to make the path absolute: $LIBDIR/lib And LIBDIR is set to /home/me/git/numbercruncher Then I expect the environment variable to be expanded in the watch path: /home/me/git/numbercruncher/lib. Instead I see the error, Could not enable auto-restart: $LIBDIR/lstat lib: no such file or directory.

bradobro commented 7 years ago

Would you consider a PR to change both of the behaviors in watches?

theothertomelliott commented 7 years ago

Thanks for the detailed problem description! Would be more than happy to accept a PR.

I've put this into the 1.8.5 milestone, so any changes would need to be on the develop branch.

bradobro commented 7 years ago

Great!

Looks to me like services.ServiceConfig.Watch() is the place to do it, expanding the included and excluded paths there.

Do you have any advice or alternative approaches?

theothertomelliott commented 7 years ago

That sounds like a good candidate. Although the expansion could also be done on loading the config (see the config package).

I'd like to centralize the responsibility for these expansions, but that would probably be a bigger refactoring job, since it's currently spread out across different areas.

bradobro commented 7 years ago

I was able to get watch dirs (include and exclude) expanding just fine.

I have some questions about service paths: whether they root from the directory of the config file (how edward generate behaves) or the cwd when edward is invoked (how services/command.go behaves on the develop branch (which, incidentally, seems to break ENV expansion).

For quick-and-dirty file watchers, cwd seems fine, but for something as expansive as edward--the sort of tool I'd use to orchestrate between several repositories--treating paths as relative to the config file seems less brittle. (modd does the same thing, though, with no easy workaround.)

I don't use imports yet, but if I did, I'd want the same behavior. Right now it looks like they are set up to treat imports as relative to the config file that imports them, not the config file that defines the service.

theothertomelliott commented 7 years ago

The service paths (for invoking the service) are considered to be relative to the config file that contains them. When importing config files, Edward performs a normalization step so all the paths should be relative to the root config file in the in-memory form of a service. If it's not doing this part correctly, that's certainly a bug.

The file watcher paths would certainly be best served by being relative to the config file path. Although thus far, the testing done on these has been manual using monorepo-like environments. I'm currently working on adding integration tests for each feature of Edward, and there's definitely room for unit testing the watcher logic and configuration further so we can be sure it would perform well in your particular use case.

bradobro commented 7 years ago

Thanks, Tom. I might not have read through the normalization method well enough. I wanted to make sure I was on the same page before sending a PR.

theothertomelliott commented 7 years ago

No problem. I'd be interested to hear more about the sort of projects you'd like to run with Edward, and see what we can do to make the tool more useful for multi-repository environments like the ones you described. I've created a Gitter chat for the project if you'd like to stop by at any point and leave your thoughts.

https://gitter.im/yext-edward/Lobby