add support for external server for serving the static assets #6311
Conversation
|
|
|
cache busting is handled by the javascript file is minified and revisioned (gets a hash in it's name) |
|
@torkelo thanks, this wasn't clear for me, as I am building from source for 32bit make build will not generate the production JS, I had to dig into the grunt file to finally find the grunt release task. I uploaded a new version without the cacheBusting fix. |
| System.config({ | ||
| defaultJSExtenions: true, | ||
| baseURL: 'public', | ||
| baseURL: getBaseURL(), |
There was a problem hiding this comment.
I am not sure if this change is necessary.
Do the plugin system uses this? in this case this is necessary. If its only the config for the boot.js generation, then I will remove the change.
Please advise.
There was a problem hiding this comment.
you cannot change baseURL in system.conf some things are loaded async (plugins and some sections)
There was a problem hiding this comment.
Dont we want to load those things from the static server? In this case, we need to have system.conf point to the static server.
So for the build, we need it to point to local filesystem 'public', but for the production, we need it to point to the static server.
There was a problem hiding this comment.
non core plugins are fetched via a special route public/plugins/ so this approach wont work.
There is still the need to setup CORS in the static server as system.js is fetching the scripts via xhr. Using scriptLoad: true will not work here (yet) as it needs to have much more metadata.
|
@torkelo is there anything I need to do for this PR to be merged? |
| For optimization you can put those files on a host optimized for serving static | ||
| files (like a CDN or S3 like server). | ||
|
|
||
| > **Note** You will need to setup CORS headers on your static server to make this work |
There was a problem hiding this comment.
Bit confused about line :) the CORS headers would be needed on the grafana-server http api, not the static server
There was a problem hiding this comment.
Yes.
Because system.js loads its JS using XHR, then the static server must have cors configured to enable XHR from the grafana server URL
| @@ -172,7 +173,7 @@ func init() { | |||
| logger = log.New("settings") | |||
| } | |||
|
|
|||
| func parseAppUrlAndSubUrl(section *ini.Section) (string, string) { | |||
| func parseAppUrlAndSubUrl(section *ini.Section) (string, string, string) { | |||
There was a problem hiding this comment.
this seems a bit strange, why mingle the public url parsing with the static url?
There was a problem hiding this comment.
It is for generating the default value, which is based on the public_url
There was a problem hiding this comment.
but seems strange to have that in this function, or am I missing something?
- remove setup of static_url in system.conf.js after discussion, this is not straightforward to load the plugins from a static server - remove references to CORS. As system.js will not load anything anymore in production mode, there is no need to setup cors. - mention that the plugins are still served by the main server - refactor the setting code to parse static_url apart from the AppUrl
|
I updated the code to address the review comments |
|
Have you tested to install external plugins, does it work? |
1 similar comment
|
Have you tested to install external plugins, does it work? |
|
@torkelo I have not yet tested the installed plugin |
|
created feature request for this issue, #6844 Closing this for now to gauge interest. |
|
interesting, would help speed up load times. |
Implementation for #6307
There is still the need to setup CORS in the static server as system.js is fetching
the scripts via xhr.
Using scriptLoad: true will not work here (yet) as it needs to have much more metadata.
This also adds a better approach for cacheBusting (using the git commit id, instead of Date.now())