Vojtech Szocs has posted comments on this change.
Change subject: PXE boot UI plugin.
......................................................................
Patch Set 3:
Nice plugin overall!
> Yeah, a readme file describing the plugin and its usability would be great.
+1
Some more questions/comments:
1. In ui-vm-pxe.json, "config.url" represents the base URL of Engine REST API,
users can (optionally) customize this value via "config.url" in
/etc/ovirt-engine/ui-plugins/ui-vm-pxe-config.json. In practice, users always
need to customize this value (by hand) according to their Engine deployment.
I think we could improve this by providing API function that returns such base
URL at runtime, for example:
var conf = api.configObject();
var restApiBaseUrl = conf.restApiBaseUrl(); // not implemented yet!
alert(restApiBaseUrl); // http(s)://<engine>/api
What do you think?
2. Although most browsers support XMLHttpRequest (including IE7+), you can
consider using XHR abstractions such as jQuery $.ajax() function instead of
working with XMLHttpRequest directly.
Since WebAdmin supports IE9+ (running in IE8 is possible, but slow as hell),
it's not a big deal, though.
3. Due to how Engine REST API integration works today (i.e. WebAdmin first
acquires session by making GET to /api upon successful login and browser
receives JSESSIONID cookie for /api within response), if I'm not mistaken, you
could remove "xmlhttp.setRequestHeader('cookie', 'JSESSIONID=' + session);" and
it would still work. I'm assuming this because JSESSIONID cookie is already set
for /api through initial "acquire session" response.
Anyway, the above is just for information, setting JSESSIONID cookie for /api
should be done anyway.
4. Small thing, "baseurl = conf.url;" could be done right after "conf =
api.configObject();" :-)
--
To view, visit http://gerrit.ovirt.org/15498
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I23264376bae9786bf9a3b291290f580bca7261c8
Gerrit-PatchSet: 3
Gerrit-Project: samples-uiplugins
Gerrit-Branch: master
Gerrit-Owner: Lee Yarwood <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Lee Yarwood <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
Gerrit-HasComments: No
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
