On 11/26/2011 07:49 PM, Geert Jansen wrote:
i just wrote a small program with the new Python SDK. I found it very easy to use, so
great work. I do have a few points of feedback:
* Probably we want all Exceptions derived from a single base class. That way, the user
can intercept any ovirtsdk with a single try/except clause. Currently that's not
possible, as all your exceptions derive directly from Exception.
sure, in my TODO list.
* I think the .get() method should be id-based not name based. Using "id" as a
primary key has a number of advantages over name:
- It is immutable, which makes e.g. renaming a lot easier.
- It allows you to retrieve an object without a search query or a filter.
- It matches how the API works as well.
"id" is supported, you can do api.collection.get(id=xxx), only as you mentioned
"id" is not primary, but secondary key (i thought that searching by name will be
but i can change it in a second if this is an issue.
Also currently i think it's not possible to retrieve a resource by its ID, unless
you use the id= keyword argument to list() which retrieves the whole collection and then
filters out the matching id.
nope, it's supported - i even added it to examples.py 
-> vm5 = api.vms.get(id='02f0f4a4-9738-4731-83c4-293f3f734782')
 will add it to get method .get(id, name) as you suggested.
* You may want to introduce some non-entity related operations, like ping(), and
connect(). These allow you to better control when you connect, and allow an program to
test if the connection details work. That helps client except the right exceptions at the
right time, and display more relevant error messages.
- no need for explicit connect() as its done implicitly on proxy construction
- no need for ping() as any attempt to use disconnected (and not recoverable) proxy, will
so user should only /Trap/ this exception for disaster_recovery
* There's two Python objects involved for each API object. For a VM for example, you
have params.VM and brokers.VM. This distinction is made visible to the user, for example:
template = api.templates.get('Blank')
vm = params.VM()
vm.name = 'foo'
vm.template = template # ERROR
yep, you right doing this is in my TODO list as well
(had to switch to CLI, so i do have few leftovers - this is one of them)
 they all have simple workarounds (like this one as described below)
The last line need to be:
vm.template = params.Template(id=template.id)
for now (as workaround you can pass 'superclass', e.g vm.template =
but i suggest you just wait for another day till I'll support this.
In my version of the Python API i used mix-in classes to add the OO behavior to the
generated classes. It was easy, because PyXB had support for that. This gave me the
single class behavior. I am not sure if GenerateDS generated code allows that. I'm
pretty sure it could be done though, but you may need to do some Python magic.
no need, i do this with regular inheritance pattern
* Nitpick: should this just be called "ovirt" instead of "ovirtsdk"?
initially it was, but since both cli & sdk would operate in same namespace,
i wanted to avoid future collisions and renamed them to ovirtsdk/ovirtcli.
Overall great work though!
10nx a lot mate.
Engine-devel mailing list
RedHat, ENG-Virtualization R&D