Isilon / isilon_sdk

Official repository for isilon_sdk. All language bindings are available for download under the 'Releases' tab.
MIT License
50 stars 21 forks source link

Adds configure() for picking best version of sdk. #3

Closed apecoraro closed 7 years ago

apecoraro commented 8 years ago

I added the configure() function in order to make it easy to figure out which version of the Python bindings to import in order to talk to a specific Isilon cluster's PAPI service.

apecoraro commented 8 years ago

I was gonna do something like this in order to get the Isilon Data Insights Connector working with the new version of the isi_sdk and I figured other people might find it useful. Example of the way it would work:

import isilon_sdk
# dynamically figure out which version of the sdk i need to be using to talk to vnode2294
api_client, isi_sdk, version = isilon_sdk.configure("vnode2294.west.isilon.com", "root", "a")
exports = isi_sdk.ProtocolsApi(api_client).list_nfs_exports()
print exports

Let me know what you think.

cbrainerd commented 8 years ago

I think this could be handy to have, but it might need a bit of explanation around the SDK version differences. IE, if you use this you should either limit the endpoints used to those that don't change between versions, or you'll need to handle the differences in your code.

apecoraro commented 8 years ago

Alright, I made a few changes to the code - fixed the issue with the hard coded auth credentials, also broke out the multi-thread/client safe implementation of ApiClient into separate class because I figured if people don't want to use the configure() function they still might want to be able to use the SDK in multi-thread/client scenarios.

apecoraro commented 8 years ago

The way that the new IsiApiClient classes work:

import isi_sdk_8_0
import isilon_sdk
client1 = isilon_sdk.IsiApiClient_8_0(host="vnode2294", verify_ssl=False)
client2 = isilon_sdk.IsiApiClient_8_0(host="vnode3394", verify_ssl=False)
# note that if you used the isi_sdk_8_0.ApiClient like this then the request would go to vnode3394
# instead of vnode2294, but with IsiApiClient_8_0 it works as expected.
isi_sdk_8_0.ProtocolsApi(client1).list_nfs_exports()
# now query vnode3394
isi_sdk_8_0.ProtocolsApi(client2).list_nfs_exports()
cbrainerd commented 8 years ago

LGTM

apecoraro commented 8 years ago

I'm thinking that I should probably add some documentation to the README.md for this stuff. @cbrainerd are you the primary owner of this repo, just wondering because I'm curious if @bishopw or @dmoxon want to weigh in on this change and whether or not it jibes with the plans for this repo.

cbrainerd commented 8 years ago

I'd say @dmoxon is the primary

apecoraro commented 8 years ago

The other thing that is missing from this is some unit test code, but I'm not exactly sure where to put that. Please advise :)

apecoraro commented 8 years ago

Also, it occurred to me yesterday that this code might work better as it's own project - isilon_sdk_utils or something like that.

bishopw commented 8 years ago

I agree that this patch should add explanations of using the IsiApiClient_8_0/IsiApiClient_7_2 classes to the README.md before we merge. Sounds like you are going to be away for a couple of weeks, but it seems okay to wait on this one. Feel free to update the README however sounds best to you when you get back. I don't see why we would spin off a whole new project for these though.

bishopw commented 8 years ago

For tests, I think you want to plug them in to the post-build step that Chris set up, so ask him about that. I think the test code would go in our internal project at [internal github]/eng-axp/sdk-test.