Improve GUI handler. #7290

Merged
merged 1 commit into from May 2, 2017

Conversation

Projects
None yet
5 participants
Member

frankban commented Apr 27, 2017

Avoid using too much logic on the server side, and support all URLs starting from /gui/.
The GUI knows how to handle model connections based on the path.
This change fixes some weird issues in which some valid paths from the perspective of the GUI (for instance /u/{user}, the profile page) were not valid from the handler perspective, resulting in errors after refreshing.
This branch also simplifies the URLs for static files to no longer assume there is always a model connected.

QA:

  • bootstrap a controller (for instance by running juju bootstrap lxd local --config identity-url=https://api.jujucharms.com/identity --build-agent);
  • run juju gui --browser, then visit the GUI, log in and check that everything works correctly;
  • visit the profile page and refresh: check that the GUI loads correctly on the profile page again;
  • switch models and refresh: check that the GUI loads correctly on the model you switched to;
  • try to visit directly an external user page, like /gui/u/bac;
  • use juju show-model | grep model-uuid to find your current model UUID, copy that and use that to make a request to the legacy GUI URL, like /gui/753a800d-80de-4d13-8552-e972376b5946: ensure everything works well in that case, and the GUI handler still handles correctly requests from old Juju clients; the whole URL is considered the base for the GUI, as it used to be;
  • run juju upgrade-gui 2.2.0 in order to install an old version of the GUI, which didn't support the new "/u/user/model" paths;
  • run juju gui --browser again, and check that even this old version works.
Improve GUI handler.
Avoid using too much logic on the server side, and support all URLs starting from /gui/.
The GUI knows how to handle model connections based on the path.
This change fixes some weird issues in which some valid paths from the perspective of the GUI (for instance /u/{user}, the profile page) were not valid from the handler perspective, resulting in errors after refreshing.
This branch also simplifies the URLs for static files to no longer assume there is always a model connected.

Thanks @frankban

}
// serveStatic serves the GUI static files.
func (h *guiHandler) serveStatic(w http.ResponseWriter, req *http.Request) {
+ logger.Debugf("serving Juju GUI static files")
@hatched

hatched Apr 28, 2017

Owner

should this not print out the path it's serving as well? Also for the other new serve logs below

Owner

hatched commented Apr 28, 2017

QA OK
Thanks @frankban

Member

frankban commented Apr 28, 2017

$$merge$$

Contributor

jujubot commented Apr 28, 2017

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

Member

axw commented May 1, 2017

$$merge$$

Contributor

jujubot commented May 1, 2017

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

Contributor

jujubot commented May 1, 2017

Build failed: Generating tarball failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/10798

Member

axw commented May 1, 2017

$$merge$$

Contributor

jujubot commented May 1, 2017

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

Contributor

jujubot commented May 1, 2017

Build failed: Tests failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/10803

Member

axw commented May 1, 2017

$$merge$$

Contributor

jujubot commented May 1, 2017

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

Contributor

jujubot commented May 1, 2017

Build failed: Tests failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/10808

Member

frankban commented May 2, 2017

$$merge$$

Contributor

jujubot commented May 2, 2017

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit ba727ca into juju:develop May 2, 2017

1 check passed

github-check-merge-juju Built PR, ran unit tests, and tested LXD deploy. Use !!.*!! to request another build. IE, !!build!!, !!retry!!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment