auxesis / visage

Graph collectd metrics in the browser, backed by a JSON API
http://visage-app.com/
Other
381 stars 56 forks source link

Make CONFIG_PATH nginx compatible. #70

Open brodock opened 13 years ago

brodock commented 13 years ago

There is no way to define ENV['CONFIG_PATH'] on nginx, but passenger has a similar way:

passenger_set_cgi_param CONFIG_PATH "/var/lib/visage";

But by this way, you define the request.env not the "system environment". This leads to the problem that Visage has to try to load profiles only after the first HTTP resquest (and not preloaded on application startup).

I also crafter a "vhost" example to be used with nginx + passenger:

server { listen 80; server_name visage.domain.com; root /home/visage/.gems/visage-app-0.9.6/lib/visage-app/public; # <--- be sure to point to 'public'! passenger_enabled on; passenger_set_cgi_param CONFIG_PATH "/home/visage/visage-config"; }

auxesis commented 13 years ago

Hey there, Thanks for the Nginx fragment - very useful!

That's a bummer about Nginx's environment variable stuff. I'm not really comfortable passing is CONFIG_PATH as a CGI parameter.

Have you considered running Nginx as a reverse proxy for Visage running on another port?

auxesis commented 13 years ago

To clarify why I'm not comfortable: although CONFIG_PATH is checked every time a file is loaded, it's sourced from an environment variable, rather than passed as an option. Passing it as an option will significantly complicate the current code.

That said, it's a completely valid use case that should be handled. The config system needs a bit of a redesign (which will happen for the 1.1 release), and I think it should definitely handle this case.

brodock commented 13 years ago

Thanks :)