search

swevm / scaleio-py

ScaleIO API Bindings
Apache License 2.0
13 stars 14 forks source link

Api feature groups in own classes #32

Closed swevm closed 9 years ago

swevm commented 9 years ago

Not sure all restructuring make sense. Looks simpler at least with stuff split into different files. Need to work on this a bit more as common methods are located in non optimal classes

wallnerryan commented 9 years ago

@swevm typically modularizing code is a good thing. The one scaleio.py file is pretty large. I haven't looked through commit yet but on your comment about common methods, break those out into its own common module. So other modules can reference common for code re-use.

Also be careful with breaking backward compatibility. E.g. If provisioning module now owns the creat_volume function, it would be ideal if scaleio.py could support a shim such as

(in scaleio.py)
def create_volume(*args, **kwargs):
   api.provisioning.create_volume(*args, **kwargs)

So if code referencing the old API can be compatible.

2 cents, I can try and help review/test some of this as well if you want.

swevm commented 9 years ago

Ryan, You´re absolutely right. I plan to keep it backward compatible as much as possible. Need to move this Pull over to dev-0.4 tree as its not complete yet.

wallnerryan commented 9 years ago

Thanks @swevm, only bring it up as we obviously used this library in the scaleio flocker driver, so backwards compat is good for less teeth pulling :)

Let me know if you need any help. :+1: