search

nmaupu / freenas-provisioner

Kubernetes external provisioner using Freenas as backend
Apache License 2.0
133 stars 26 forks source link

support for 11.3 #35

Closed travisghansen closed 4 years ago

travisghansen commented 4 years ago

Deal with 11.3 api craziness.

maxirus commented 4 years ago

One potential issue I thought of is there may be a char byte limit on the API making this approach not scalable. I've yet to find the API codebase for this so this is all speculative.

A solution to this would be reduce it to its lowest (largest?) units like this package.

travisghansen commented 4 years ago

We do have that package imported already so it could be used. I tend believe it's highly unlikely to hit an issue like that so I'd prefer to use bytes (since that's what is returned by the API) to keep the incoming/outgoing payloads as close as possible. Happy to have further review though for sure if we feel it's necessary.

maxirus commented 4 years ago

Ah, didn't realize the return values were in bytes. Yep, agreed; keep it similar.

travisghansen commented 4 years ago

@nmaupu what do you think on this one?

nmaupu commented 4 years ago

LGTM, I wonder if it would be better to directly change the Dataset struct directly though... https://github.com/nmaupu/freenas-provisioner/blob/master/freenas/dataset.go#L15

travisghansen commented 4 years ago

My vote would be no, as unmarshaling is actually a number so it 'just works' and it doesn't require messing with any of the 'outside' code.