search

fog / fog-libvirt

libvirt provider for fog
MIT License
16 stars 42 forks source link

Deal with no flags passed to Server#destroy #125

Closed ekohl closed 1 year ago

ekohl commented 1 year ago

In 085a7c89596e989921dccd33dd954afbb37df27a it became possible to pass flags to Server#destroy but if the caller didn't pass flags in opts it failed because it was essentially nil.zero? which isn't defined.

I wasn't sure if I should drop the defaults for flags in options, but decided to keep it in to "document" the API.

ekohl commented 1 year ago

Good question. What I'd actually prefer is def destroy(destroy_volumes: false, flags: 0, **kwargs) but that's probably breaking the API and have issues with Ruby < 2.7.

adamruzicka commented 1 year ago

Yes, that would be even better. Do we have to keep compatibility with eol rubies? It is breaking the api, but we will have to do that eventually anyway

ekohl commented 1 year ago

I don't know to be honest. I also don't know if we need to remain compatible with some generic fog interface for destroy(). If we don't, kwargs could actually be dropped.

ekohl commented 1 year ago

For now I'm going to merge this. In Foreman we now pin to >= 0.9.0 (https://github.com/theforeman/foreman/blob/339fe635ffa37d52e7174be401e9b54af20c5681/bundler.d/libvirt.rb#L2) and that means that a new release on Debian would break if we change the API. So this isn't a long term solution, but it makes the code safer.