Skip to content

Allow relocation of Brooklyn REST API#11

Merged
asfgit merged 1 commit intoapache:masterfrom
CMoH:relocate-rest-baseUrl
Mar 9, 2016
Merged

Allow relocation of Brooklyn REST API#11
asfgit merged 1 commit intoapache:masterfrom
CMoH:relocate-rest-baseUrl

Conversation

@CMoH
Copy link
Contributor

@CMoH CMoH commented Feb 29, 2016

This PR follows the proposal on the mailing list about allowing the REST API to be relocated relative to brooklyn's web UI.

For now the function is not changed, but this allows administrators to deploy Brooklyn's REST API and its UI indepentent of each other's base URLs.

// var baseURL = "/api/brooklyn/";
var baseURL = "";

if (baseURL && settings.url.match(/^\/v1\/\w+/i)){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor, .indexOf('/v1/') == 0 would be more clear

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True it would be more clear, but the way it's currently set up is to intercept only calls to URLs that begin with /v1/, so something like /v1/projects will be affected, while /api/v1/projects won't.

If you use .indexOf('/v1/') == 0, /api/v1/projects will be affected as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sliceratwork i don't think so:

% node
> '/api/v1/project'.indexOf('/v1/')
4

btw we extend the prototype of String in brooklyn-utils.js so you could just write .startsWith('/v1/').

@neykov
Copy link
Member

neykov commented Mar 1, 2016

@CMoH quite clever to plug in at this level, I like it. Do you think the code should strip the /v1 prefix, now that it's not required by the REST API?

@CMoH
Copy link
Contributor Author

CMoH commented Mar 1, 2016

Actually the credit goes to @sliceratwork - I merely wrote it on my machine.

I don't know about the /v1 prefix. This is more about allowing various configurations during deployment. I would prefer having an external config for the base URL, but this is a start.

@ahgittin
Copy link
Contributor

ahgittin commented Mar 7, 2016

i like.

in terms of future direction allowing the base url to be configurable at deploy time would be nice, though maybe difficult (?).

minor preference but in future might be cleaner to swap all the /v1/... api calls in JS to be /brooklyn/v1/... and then we replace any link starting /brooklyn/ with the new prefix

@ahgittin
Copy link
Contributor

ahgittin commented Mar 7, 2016

PS good to merge once the regex / startsWith comments are agreed

Add a function that prefixes AJAX REST requests with a configurable base
URL. The base URL is inhere left empty for backward compatibility.
@CMoH
Copy link
Contributor Author

CMoH commented Mar 8, 2016

I switched to using startsWith. Thanks for the tip

@asfgit asfgit merged commit 3b7090b into apache:master Mar 9, 2016
asfgit pushed a commit that referenced this pull request Mar 9, 2016
tommclaughlan pushed a commit to diffusiondata/brooklyn-ui that referenced this pull request Jan 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants