Change in vdsm[master]: Add the REST API bindings

Adam Litke has uploaded a new change for review.

Change subject: Add the REST API bindings
......................................................................

Add the REST API bindings

Conflicts:

vdsm/clientIF.py

Change-Id: I8b914f7ad82fee8d9e7e3ce6847ffe4cda374a56
---
M configure.ac
M vdsm.spec.in
M vdsm/BindingXMLRPC.py
M vdsm/Makefile.am
M vdsm/clientIF.py
M vdsm/config.py.in
A vdsm/rest/BindingREST.py
A vdsm/rest/Controller.py
A vdsm/rest/Dispatcher.py
A vdsm/rest/Makefile.am
A vdsm/rest/Store/LocalDB.py
A vdsm/rest/Store/Makefile.am
A vdsm/rest/Store/StorageDriver.py
A vdsm/rest/Store/__init__.py
A vdsm/rest/__init__.py
A vdsm/rest/templates/Makefile.am
A vdsm/rest/templates/image.json.x
A vdsm/rest/templates/images.json.x
A vdsm/rest/templates/root.json.x
A vdsm/rest/templates/storageconnection.json.x
A vdsm/rest/templates/storageconnections.json.x
A vdsm/rest/templates/storagedomain.json.x
A vdsm/rest/templates/storagedomains.json.x
A vdsm/rest/templates/storagepool.json.x
A vdsm/rest/templates/storagepools.json.x
A vdsm/rest/templates/task.json.x
A vdsm/rest/templates/tasks.json.x
A vdsm/rest/templates/vm.json.x
A vdsm/rest/templates/vmdrive.json.x
A vdsm/rest/templates/vmdrives.json.x
A vdsm/rest/templates/vms.json.x
A vdsm/rest/templates/volume.json.x
A vdsm/rest/templates/volumes.json.x
33 files changed, 1,740 insertions(+), 16 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/21/2021/1
--
To view, visit http://gerrit.ovirt.org/2021
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I8b914f7ad82fee8d9e7e3ce6847ffe4cda374a56
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke

Adam Litke has posted comments on this change.

Change subject: Add the REST API bindings
......................................................................

Patch Set 1: I would prefer that you didn't submit this

This is just an RFC and not ready to push yet.  Please review!  Thanks!

--
To view, visit http://gerrit.ovirt.org/2021
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8b914f7ad82fee8d9e7e3ce6847ffe4cda374a56
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Add the REST API bindings
......................................................................

Patch Set 1: (5 inline comments)

....................................................
Commit Message
Line 7: Add the REST API bindings
some claim that Python is clearer than English, but I would prefer a short introduction of the new capability, a sketch of its architecture, and an explanation of its use.

....................................................
File vdsm/config.py.in
Line 227: 			'clients.'),
unexpanded tabs?! the horror.

....................................................
File vdsm/rest/Controller.py
Line 62: class restException(Exception):
Class starting with a lower case? (don't copy the bad things we have)

Line 76:     print d
logging?

....................................................
File vdsm/rest/Store/LocalDB.py
Line 21: class LocalDB(StorageDriver):
is this file intentionally included?

I would prefer to have a separate discussion regarding local DB, as the concept conflicts with that of ovirt-Engine-owned DB.

--
To view, visit http://gerrit.ovirt.org/2021
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8b914f7ad82fee8d9e7e3ce6847ffe4cda374a56
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg

Adam Litke has posted comments on this change.

Change subject: Add the REST API bindings
......................................................................

Patch Set 1: (3 inline comments)

....................................................
File vdsm/config.py.in
Line 227: 			'clients.'),
Grr. Eclipse.  Will fix.

....................................................
File vdsm/rest/Controller.py
Line 62: class restException(Exception):
Done

....................................................
File vdsm/rest/Store/LocalDB.py
Line 21: class LocalDB(StorageDriver):
Yes, correct.  I did not mean to include this file in this series.

--
To view, visit http://gerrit.ovirt.org/2021
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8b914f7ad82fee8d9e7e3ce6847ffe4cda374a56
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg

Adam Litke has posted comments on this change.

Change subject: Add the REST API bindings
......................................................................

Patch Set 2:

Test comment -- please ignore.

--
To view, visit http://gerrit.ovirt.org/2021
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8b914f7ad82fee8d9e7e3ce6847ffe4cda374a56
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Mark Wu

Mark Wu has posted comments on this change.

Change subject: Add the REST API bindings
......................................................................

Patch Set 2:

Wenchao,
I think you should point out the bugs or file it to internal bugzilla and then Adam could make a new patch set with the fix.

--
To view, visit http://gerrit.ovirt.org/2021
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8b914f7ad82fee8d9e7e3ce6847ffe4cda374a56
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Wenchao Xia

Wenchao Xia has posted comments on this change.

Change subject: Add the REST API bindings
......................................................................

Patch Set 2: Looks good to me, but someone else must approve

Reviewed and Tested it to create VMs. Although there are minor bugs in this version, but pushing it now may let other people help resolve them.

--
To view, visit http://gerrit.ovirt.org/2021
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8b914f7ad82fee8d9e7e3ce6847ffe4cda374a56
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Wenchao Xia

Royce Lv has posted comments on this change.

Change subject: Add the REST API bindings
......................................................................

Patch Set 2: (2 inline comments)

nit bugs as I mentioned in the inline comments,other parts looks good to me.

....................................................
File vdsm/rest/Controller.py
Line 608:         raise cherrypy.HTTPRedirect(url, 303)
After format 303 redirect to /storagedomains/uuid/ will result in 404 error,it will fail format operation,we can return 204 instead.

Line 837:                 pool.obj._UUID, 303)
Same as this place,redirect will return 404 because storage pool is not connected,so we can't get back pool definition,here

--
To view, visit http://gerrit.ovirt.org/2021
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8b914f7ad82fee8d9e7e3ce6847ffe4cda374a56
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Wenchao Xia

Change in vdsm[master]: rest-api: Add the basic server infrastructure

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: rest-api: Add the basic server infrastructure
......................................................................

Patch Set 3: (8 inline comments)

partial review

....................................................
Commit Message
Line 39:  * Cheetah template files to render responses in json and xml format
do we really need both?
is there no automatic translation between xml to json?
I fear data duplication.

Line 50: Signed-off-by: Adam Litke 
what tests/rest_override/__init__.py is for?

vdsm/rest subdir should probably be added to pep8's whitelist. and to pep8's.
and a real pyflakes issue should be fixed:

tests/restTests.py:57: local variable 'IndexError' is assigned to but never used

....................................................
File vdsm/clientIF.py
Line 99:     def _getServerIP(self, addr=None):
eh, that's back here. I have a deja vu ;-)

....................................................
File vdsm/rest/BindingREST.py
Line 24:     def __init__(self, cif, log, params):
wouldn't it be more pythonic to have two optional args instead of this 'params' dict?

....................................................
File vdsm/rest/Controller.py
Line 52:         templatePath = "%s/rest/templates" % constants.P_VDSM
Saggi would ask you to put this default up in clientIF and not pollute your new module with vdsm.constants; I may just beat him to it.

....................................................
File vdsm/rest/Dispatcher.py
Line 1: import cherrypy
GPL boilerplate required, sorry :-(

Line 37:             # Decode any leftover %2F in the virtual_path atoms.
I haven't looked deeply, but what do you mean by "leftover"? Usually, uri decoding should be done once, and completely - if %25 has been decoded already, you may be decoding a literal %2F, not a slash.

re-implementing decode has plenty of pitfalls. rfc 1738 says:

(The characters "abcdef" may also be used in hexadecimal encodings.)

....................................................
File vdsm.spec.in
Line 45: Requires: python-cherrypy python-cheetah
Have you considered separating this into another sub-rpm?

I think we wanted to factor out xmlrpc, too?

--
To view, visit http://gerrit.ovirt.org/2021
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8b914f7ad82fee8d9e7e3ce6847ffe4cda374a56
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Wenchao Xia

Change in vdsm[master]: rest-api: Add the basic server infrastructure

Adam Litke has posted comments on this change.

Change subject: rest-api: Add the basic server infrastructure
......................................................................

Patch Set 3: (7 inline comments)

....................................................
Commit Message
Line 50: Signed-off-by: Adam Litke 
Yes, now that I understand how the pep8 whitelist works, I will add some files and fix any problems.

....................................................
File vdsm/clientIF.py
Line 99:     def _getServerIP(self, addr=None):
Yep.  The same logic applies to multiple bindings so I want to avoid duplicating the code.

....................................................
File vdsm/rest/BindingREST.py
Line 24:     def __init__(self, cif, log, params):
I wasn't sure how many args would eventually be needed and I wanted to keep the initialization of this binding consistent with how BindingXMLRPC is done.

....................................................
File vdsm/rest/Controller.py
Line 52:         templatePath = "%s/rest/templates" % constants.P_VDSM
Ok, so pass this into the BindingREST constructor?  That is no problem at all.

....................................................
File vdsm/rest/Dispatcher.py
Line 1: import cherrypy
Done

Line 37:             # Decode any leftover %2F in the virtual_path atoms.
This part comes straight from the cherrypy default dispatcher.  My goal was to change the default dispatcher only as much as is necessary to achieve the desired behavior.

....................................................
File vdsm.spec.in
Line 45: Requires: python-cherrypy python-cheetah
Would it be good enough to allow disabling the rest bindings in vdsm.conf?

--
To view, visit http://gerrit.ovirt.org/2021
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8b914f7ad82fee8d9e7e3ce6847ffe4cda374a56
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Wenchao Xia

Change in vdsm[master]: rest-api: Add the basic server infrastructure

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: rest-api: Add the basic server infrastructure
......................................................................

Patch Set 3: (3 inline comments)

....................................................
File vdsm/rest/BindingREST.py
Line 24:     def __init__(self, cif, log, params):
I agree that the same problem applies to BindingXMLRPC: python's way of passing initially-unknown arguments is with **kwargs.

....................................................
File vdsm/rest/Controller.py
Line 52:         templatePath = "%s/rest/templates" % constants.P_VDSM
cool - in case it was not clear, my point is to make the code more easily testable, and avoid set_template_path() completely.

....................................................
File vdsm.spec.in
Line 45: Requires: python-cherrypy python-cheetah
hmm, maybe. since your code is in its own python package, it would not incur much harm if shipped disabled. on the other hand, we do want to become more modular.

--
To view, visit http://gerrit.ovirt.org/2021
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8b914f7ad82fee8d9e7e3ce6847ffe4cda374a56
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Wenchao Xia

Change in vdsm[master]: rest-api: Add the basic server infrastructure

ShaoHe Feng has posted comments on this change.

Change subject: rest-api: Add the basic server infrastructure
......................................................................

--
To view, visit http://gerrit.ovirt.org/2021
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8b914f7ad82fee8d9e7e3ce6847ffe4cda374a56
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: ShaoHe Feng 
Gerrit-Reviewer: Wenchao Xia

Change in vdsm[master]: rest-api: Add the basic server infrastructure

Saggi Mizrahi has posted comments on this change.

Change subject: rest-api: Add the basic server infrastructure
......................................................................

Patch Set 4: Looks good to me, but someone else must approve

If you have another iteration:
- make sure the years on the copyright notices are 2012
- You have a mix of tabs and spaces in some of the xml templates please keep consistent indentation.

Other then that superb job

--
To view, visit http://gerrit.ovirt.org/2021
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8b914f7ad82fee8d9e7e3ce6847ffe4cda374a56
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: ShaoHe Feng 
Gerrit-Reviewer: Wenchao Xia

Change in vdsm[master]: rest-api: Add the basic server infrastructure

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: rest-api: Add the basic server infrastructure
......................................................................

Patch Set 3:

annoying comment (sorry, there are lawyers in the audience)

--
To view, visit http://gerrit.ovirt.org/2021
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8b914f7ad82fee8d9e7e3ce6847ffe4cda374a56
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: ShaoHe Feng 
Gerrit-Reviewer: Wenchao Xia

Change in vdsm[master]: rest-api: Add the basic server infrastructure

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: rest-api: Add the basic server infrastructure
......................................................................

Patch Set 4: I would prefer that you didn't submit this

(1 inline comment)

....................................................
File vdsm/rest/Dispatcher.py
Line 1: import cherrypy
I'm still missing my cherished boilerplate :-(

--
To view, visit http://gerrit.ovirt.org/2021
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8b914f7ad82fee8d9e7e3ce6847ffe4cda374a56
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: ShaoHe Feng 
Gerrit-Reviewer: Wenchao Xia

Change in vdsm[master]: rest-api: Add the basic server infrastructure

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: rest-api: Add the basic server infrastructure
......................................................................

Patch Set 4: (1 inline comment)

....................................................
File vdsm/rest/Dispatcher.py
Line 28:     This is the default, built-in dispatcher for CherryPy.
Actually, it is a tweaked version threreof.

hmm, if this is mostly cherrypy BSD code, we may not infect it with GPL. In any case, some copyright bullsh*t statement should appear at the top - particularly if it is different from the default GPL one.

--
To view, visit http://gerrit.ovirt.org/2021
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8b914f7ad82fee8d9e7e3ce6847ffe4cda374a56
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: ShaoHe Feng 
Gerrit-Reviewer: Shu Ming 
Gerrit-Reviewer: Wenchao Xia

Change in vdsm[master]: rest-api: Add the basic server infrastructure

Adam Litke has posted comments on this change.

Change subject: rest-api: Add the basic server infrastructure
......................................................................

Patch Set 4: (1 inline comment)

....................................................
File vdsm/rest/Dispatcher.py
Line 28:     This is the default, built-in dispatcher for CherryPy.
Ok.  Is the vdsm project okay with BSD licensed code (for this file)?

--
To view, visit http://gerrit.ovirt.org/2021
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8b914f7ad82fee8d9e7e3ce6847ffe4cda374a56
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: ShaoHe Feng 
Gerrit-Reviewer: Shu Ming 
Gerrit-Reviewer: Wenchao Xia

Change in vdsm[master]: rest-api: Add the basic server infrastructure

Adam Litke has posted comments on this change.

Change subject: rest-api: Add the basic server infrastructure
......................................................................

Patch Set 5:

Added the copyright notice to Dispatcher.py

--
To view, visit http://gerrit.ovirt.org/2021
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8b914f7ad82fee8d9e7e3ce6847ffe4cda374a56
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: ShaoHe Feng 
Gerrit-Reviewer: Shu Ming 
Gerrit-Reviewer: Wenchao Xia

Change in vdsm[master]: rest-api: Add the basic server infrastructure

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: rest-api: Add the basic server infrastructure
......................................................................

--
To view, visit http://gerrit.ovirt.org/2021
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8b914f7ad82fee8d9e7e3ce6847ffe4cda374a56
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: ShaoHe Feng 
Gerrit-Reviewer: Shu Ming 
Gerrit-Reviewer: Wenchao Xia

Change in vdsm[master]: rest-api: Add the basic server infrastructure

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: rest-api: Add the basic server infrastructure
......................................................................

Patch Set 5: (2 inline comments)

....................................................
File vdsm/rest/Dispatcher.py
Line 2: # All rights reserved.
IANAL, but I think there's any problem with a BSD file in Vdsm. GPL infects it anyway.

But don't you own your tweaks for it? shouldn't you state it, too?

oh, and there's whitespace noise.

Line 54:     This is the default, built-in dispatcher for CherryPy.
don't you want to change the docstring? it slightly lies...

--
To view, visit http://gerrit.ovirt.org/2021
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8b914f7ad82fee8d9e7e3ce6847ffe4cda374a56
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: ShaoHe Feng 
Gerrit-Reviewer: Shu Ming 
Gerrit-Reviewer: Wenchao Xia

Change in vdsm[master]: rest-api: Add the basic server infrastructure

Adam Litke has posted comments on this change.

Change subject: rest-api: Add the basic server infrastructure
......................................................................

Patch Set 5: (1 inline comment)

Made the requested changes.

....................................................
File vdsm/rest/Dispatcher.py
Line 2: # All rights reserved.
ok, will add a copyright statement for myself and clean the whitespace.  What needs to happen from a legal POV to approve this?  Anything?  We meet the requirements because this file is always distributed so the BSD message will always tag along to the installed system.

--
To view, visit http://gerrit.ovirt.org/2021
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8b914f7ad82fee8d9e7e3ce6847ffe4cda374a56
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: ShaoHe Feng 
Gerrit-Reviewer: Shu Ming 
Gerrit-Reviewer: Wenchao Xia

Change in vdsm[master]: rest-api: Add the basic server infrastructure

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: rest-api: Add the basic server infrastructure
......................................................................

Patch Set 5: (1 inline comment)

....................................................
File vdsm/rest/Dispatcher.py
Line 2: # All rights reserved.
ianal, but I think that we're in the clear.

hmmm, ovirt-node drops .py and keeps only .pyc files. To for the text to stay on disk there, it should be a docstring, I'm afraid.

--
To view, visit http://gerrit.ovirt.org/2021
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8b914f7ad82fee8d9e7e3ce6847ffe4cda374a56
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: ShaoHe Feng 
Gerrit-Reviewer: Shu Ming 
Gerrit-Reviewer: Wenchao Xia

Change in vdsm[master]: rest-api: Add the basic server infrastructure

Adam Litke has posted comments on this change.

Change subject: rest-api: Add the basic server infrastructure
......................................................................

Patch Set 5:

Changed the BSD message to a docstring.

--
To view, visit http://gerrit.ovirt.org/2021
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8b914f7ad82fee8d9e7e3ce6847ffe4cda374a56
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: ShaoHe Feng 
Gerrit-Reviewer: Shu Ming 
Gerrit-Reviewer: Wenchao Xia

Change in vdsm[master]: rest-api: Add the basic server infrastructure

ShaoHe Feng has posted comments on this change.

Change subject: rest-api: Add the basic server infrastructure
......................................................................

Patch Set 6: Fails; I would prefer that you didn't submit this

(1 inline comment)

vdsm/clientIF.py:45: redefinition of unused 'blkid' from line 42
make[2]: *** [check-local] Error 1

....................................................
File vdsm/clientIF.py
Line 45: import blkid
import blkid twice.

--
To view, visit http://gerrit.ovirt.org/2021
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8b914f7ad82fee8d9e7e3ce6847ffe4cda374a56
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: ShaoHe Feng 
Gerrit-Reviewer: Shu Ming 
Gerrit-Reviewer: Wenchao Xia

Change in vdsm[master]: rest-api: Add the basic server infrastructure

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: rest-api: Add the basic server infrastructure
......................................................................

Patch Set 7:

tests/rest_override/__init__.py is noise, I suppose?

--
To view, visit http://gerrit.ovirt.org/2021
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8b914f7ad82fee8d9e7e3ce6847ffe4cda374a56
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: ShaoHe Feng 
Gerrit-Reviewer: Shu Ming 
Gerrit-Reviewer: Wenchao Xia

Change in vdsm[master]: rest-api: Add the basic server infrastructure

Adam Litke has posted comments on this change.

Change subject: rest-api: Add the basic server infrastructure
......................................................................

Patch Set 7:

argh... Indeed it is.

--
To view, visit http://gerrit.ovirt.org/2021
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8b914f7ad82fee8d9e7e3ce6847ffe4cda374a56
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: ShaoHe Feng 
Gerrit-Reviewer: Shu Ming 
Gerrit-Reviewer: Wenchao Xia

Change in vdsm[master]: rest-api: Add the basic server infrastructure

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: rest-api: Add the basic server infrastructure
......................................................................

Patch Set 7: I would prefer that you didn't submit this

I wanted to take this patch with the noise bit removed, but it required non-zero rebasing work - it would be safer if you do it.

--
To view, visit http://gerrit.ovirt.org/2021
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8b914f7ad82fee8d9e7e3ce6847ffe4cda374a56
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: ShaoHe Feng 
Gerrit-Reviewer: Shu Ming 
Gerrit-Reviewer: Wenchao Xia

Change in vdsm[master]: rest-api: Add the basic server infrastructure

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: rest-api: Add the basic server infrastructure
......................................................................

Patch Set 8: Verified; Looks good to me, approved

--
To view, visit http://gerrit.ovirt.org/2021
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8b914f7ad82fee8d9e7e3ce6847ffe4cda374a56
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: ShaoHe Feng 
Gerrit-Reviewer: Shu Ming 
Gerrit-Reviewer: Wenchao Xia

Change in vdsm[master]: rest-api: Add the basic server infrastructure

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: rest-api: Add the basic server infrastructure
......................................................................

rest-api: Add the basic server infrastructure

Add a new API binding for a REST API based on CherryPy and the Cheetah
templating framework.  This patch introduces the basic infrastructure for the
API as follows:

vdsm/clientIF.py
 * Initialize the REST API along with the XMLRPC binding.
 * The rest server is started in its own thread.  It will create more threads as
   needed.

vdsm/rest/BindingREST.py
 * REST API initialization

vdsm/rest/Controller.py
 * Main API logic controller
 * In this patch we introduce utilities and structure
 * Render the Root resource in the API

vdsm/rest/Dispatcher.py
 * A custom cherrypy URI dispatcher that handles URIs with nested identifiers:
    http://<server>:<port>/api/storagedomains/<sdUUID>/images/<imgUUID>

vdsm/rest/templates/api.xsd
vdsm/rest/templates/rsdl.xml
 * Define the rest API schema and valid URIs
 * This API is compatible with ovirt-engine-sdk and ovirt-engine-cli

vdsm/rest/templates/response.json.x
vdsm/rest/templates/response.xml.x
vdsm/rest/templates/root.json.x
vdsm/rest/templates/root.xml.x
 * Cheetah template files to render responses in json and xml format
 * The API can accept requests in either json or xml

API Implemented:
----------------
/api : The Root of the API
 * Provides vdsm version information
 * Lists the available sub-collections
 Actions: None

Change-Id: I8b914f7ad82fee8d9e7e3ce6847ffe4cda374a56
Signed-off-by: Adam Litke 
---
M Makefile.am
M configure.ac
A tests/rest_override/__init__.py
M vdsm.spec.in
M vdsm/BindingXMLRPC.py
M vdsm/Makefile.am
M vdsm/clientIF.py
M vdsm/config.py.in
A vdsm/rest/BindingREST.py
A vdsm/rest/Controller.py
A vdsm/rest/Dispatcher.py
A vdsm/rest/Makefile.am
A vdsm/rest/__init__.py
A vdsm/rest/templates/Makefile.am
A vdsm/rest/templates/api.xsd
A vdsm/rest/templates/response.json.x
A vdsm/rest/templates/response.xml.x
A vdsm/rest/templates/root.json.x
A vdsm/rest/templates/root.xml.x
A vdsm/rest/templates/rsdl.xml
20 files changed, 1,126 insertions(+), 43 deletions(-)

Approvals:
  Dan Kenigsberg: Verified; Looks good to me, approved

--
To view, visit http://gerrit.ovirt.org/2021
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I8b914f7ad82fee8d9e7e3ce6847ffe4cda374a56
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: ShaoHe Feng 
Gerrit-Reviewer: Shu Ming 
Gerrit-Reviewer: Wenchao Xia