ManageIQ / azure-armrest

Ruby interface for Azure using the new REST API
Apache License 2.0
15 stars 36 forks source link

Fix StorageAccountService#list_private_images #345

Closed NickLaMuro closed 6 years ago

NickLaMuro commented 6 years ago

When speeding up the list_all_private_images method, the list_all method was updated with the skip_accessors_definition boolean flag to that and associated methods. This was not, though, applied to the StorageAccountService#list_private_images method, which works on just a single storage account instead of all of them.

This updates #list in both the StorageAccountService version, and the base class to allow it to support this flag.

Links

miq-bot commented 6 years ago

Checked commit https://github.com/NickLaMuro/azure-armrest/commit/bb60672ef9ffc833cee8f8a0d1f385ba8eaaf8c3 with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0 2 files checked, 0 offenses detected Everything looks fine. :+1:

bronaghs commented 6 years ago

@djberg96 - Can you test this by provisioning an instance? also, can we add some specs?

NickLaMuro commented 6 years ago

also, can we add some specs?

So this is what I was hoping to do as well, but wasn't sure how we wanted to approach it. Right now, I don't think there are any tests in place that attempt to make API calls, and stubbing that kinda of stuff in my experience leads to the API changing and then tests giving false positives of passing (that said, probably better than the issue we ran into here).

Anyway, didn't want to make a decision on how we wanted to go about that in this PR, so I didn't. That said, wouldn't mind opening up another issue to discuss how we would do testing for this in the future.

djberg96 commented 6 years ago

I will add some specs in a separate PR as the code is definitely broken currently.