[Engine-devel] Feedback on ovirt-engine-sdk

[Geert Jansen]

Hi Michael,

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.

  * 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.

   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.

  * 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.

  * 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

The last line need to be:

   vm.template = params.Template(id=template.id)

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.

  * Nitpick: should this just be called "ovirt" instead of "ovirtsdk"?

Overall great work though!

Regards,
Geert