-
Notifications
You must be signed in to change notification settings - Fork 11.7k
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
add support for external server for serving the static assets #6311
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,6 +39,7 @@ var ( | |
Env string = DEV | ||
AppUrl string | ||
AppSubUrl string | ||
AppStaticUrl string | ||
InstanceName string | ||
|
||
// build | ||
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this seems a bit strange, why mingle the public url parsing with the static url? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is for generating the default value, which is based on the public_url There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. but seems strange to have that in this function, or am I missing something? |
||
appUrl := section.Key("root_url").MustString("http://localhost:3000/") | ||
if appUrl[len(appUrl)-1] != '/' { | ||
appUrl += "/" | ||
|
@@ -184,8 +185,9 @@ func parseAppUrlAndSubUrl(section *ini.Section) (string, string) { | |
log.Fatal(4, "Invalid root_url(%s): %s", appUrl, err) | ||
} | ||
appSubUrl := strings.TrimSuffix(url.Path, "/") | ||
appStaticUrl := section.Key("static_url").MustString(fmt.Sprintf("%s/public", appSubUrl)) | ||
|
||
return appUrl, appSubUrl | ||
return appUrl, appSubUrl, appStaticUrl | ||
} | ||
|
||
func ToAbsUrl(relativeUrl string) string { | ||
|
@@ -464,7 +466,7 @@ func NewConfigContext(args *CommandLineArgs) error { | |
PluginsPath = makeAbsolute(Cfg.Section("paths").Key("plugins").String(), HomePath) | ||
|
||
server := Cfg.Section("server") | ||
AppUrl, AppSubUrl = parseAppUrlAndSubUrl(server) | ||
AppUrl, AppSubUrl, AppStaticUrl = parseAppUrlAndSubUrl(server) | ||
|
||
Protocol = HTTP | ||
if server.Key("protocol").MustString("http") == "https" { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,15 @@ | ||
function getBaseURL() { | ||
"use strict"; | ||
/* in the case of system.build, window is undefined | ||
We should still pull the plugins from the CDN*/ | ||
if(typeof(window) === "undefined") { | ||
return "public"; | ||
} | ||
return window.grafanaBootData.settings.appStaticUrl || "public"; | ||
} | ||
System.config({ | ||
defaultJSExtenions: true, | ||
baseURL: 'public', | ||
baseURL: getBaseURL(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure if this change is necessary. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you cannot change baseURL in system.conf some things are loaded async (plugins and some sections) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. non core plugins are fetched via a special route public/plugins/ so this approach wont work. |
||
paths: { | ||
'remarkable': 'vendor/npm/remarkable/dist/remarkable.js', | ||
'tether': 'vendor/npm/tether/dist/js/tether.js', | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, yea your right