Use valid api-version string if settings.yml value is invalid #275
Conversation
|
Note that because this makes one extra REST API call internally, the specs will fail using the current cassettes. Those will need to be regenerated, but atm we cannot because of current internal issues. |
| "#{service.provider}/#{service.service_name} for EMS #{@ems.name}; " \ | ||
| "using '#{valid_version_string}' instead." | ||
|
|
||
| _log.warn(message) |
There was a problem hiding this comment.
Is it possible to detect this at the time it's set in the settings? I can show you where to do it...my only concern is if it makes a call out to the provider (can't tell from this code if it does or not).
There was a problem hiding this comment.
It does make one extra call internally to get that list.
There was a problem hiding this comment.
I would like to see how to do that @Fryguy
djberg96
changed the title
|
Some comments on commits https://github.com/djberg96/manageiq-providers-azure/compare/7c0f52585e5f2d3b2d7a02ee94452362ad71436d~...9f93c661d4abf55d1dd72cedd2dcaa78cac18a29 spec/vcr_cassettes/manageiq/providers/azure/cloud_manager/refresher.yml
spec/vcr_cassettes/manageiq/providers/azure/cloud_manager/refresher_inventory_object.yml
spec/vcr_cassettes/manageiq/providers/azure/cloud_manager/refresher_targeted/targeted_api_collection_threshold_0/lb_created_by_stack_refresh.yml
spec/vcr_cassettes/manageiq/providers/azure/cloud_manager/refresher_targeted/targeted_api_collection_threshold_0/lb_refresh.yml
spec/vcr_cassettes/manageiq/providers/azure/cloud_manager/refresher_targeted/targeted_api_collection_threshold_0/lb_vms_refresh.yml
spec/vcr_cassettes/manageiq/providers/azure/cloud_manager/refresher_targeted/targeted_api_collection_threshold_0/lb_with_vms_refresh.yml
spec/vcr_cassettes/manageiq/providers/azure/cloud_manager/refresher_targeted/targeted_api_collection_threshold_0/lbs_refresh.yml
spec/vcr_cassettes/manageiq/providers/azure/cloud_manager/refresher_targeted/targeted_api_collection_threshold_0/multiple_targets_refresh.yml
spec/vcr_cassettes/manageiq/providers/azure/cloud_manager/refresher_targeted/targeted_api_collection_threshold_0/network_port_refresh.yml
spec/vcr_cassettes/manageiq/providers/azure/cloud_manager/refresher_targeted/targeted_api_collection_threshold_0/powered_off_vm_refresh.yml
spec/vcr_cassettes/manageiq/providers/azure/cloud_manager/refresher_targeted/targeted_api_collection_threshold_0/powered_on_vm_refresh.yml
spec/vcr_cassettes/manageiq/providers/azure/cloud_manager/refresher_targeted/targeted_api_collection_threshold_0/vm_with_managed_disk_refresh.yml
spec/vcr_cassettes/manageiq/providers/azure/cloud_manager/refresher_targeted/targeted_api_collection_threshold_500/lb_created_by_stack_refresh.yml
spec/vcr_cassettes/manageiq/providers/azure/cloud_manager/refresher_targeted/targeted_api_collection_threshold_500/lb_refresh.yml
spec/vcr_cassettes/manageiq/providers/azure/cloud_manager/refresher_targeted/targeted_api_collection_threshold_500/lb_vms_refresh.yml
spec/vcr_cassettes/manageiq/providers/azure/cloud_manager/refresher_targeted/targeted_api_collection_threshold_500/lb_with_vms_refresh.yml
spec/vcr_cassettes/manageiq/providers/azure/cloud_manager/refresher_targeted/targeted_api_collection_threshold_500/multiple_targets_refresh.yml
spec/vcr_cassettes/manageiq/providers/azure/cloud_manager/refresher_targeted/targeted_api_collection_threshold_500/network_port_refresh.yml
spec/vcr_cassettes/manageiq/providers/azure/cloud_manager/refresher_targeted/targeted_api_collection_threshold_500/powered_off_vm_refresh.yml
spec/vcr_cassettes/manageiq/providers/azure/cloud_manager/refresher_targeted/targeted_api_collection_threshold_500/powered_on_vm_refresh.yml
spec/vcr_cassettes/manageiq/providers/azure/cloud_manager/refresher_targeted/targeted_api_collection_threshold_500/vm_with_managed_disk_refresh.yml
spec/vcr_cassettes/manageiq/providers/azure/cloud_manager/refresher_targeted_scope/lb_refresh.yml
spec/vcr_cassettes/manageiq/providers/azure/cloud_manager/refresher_targeted_scope/lb_vms_refresh.yml
spec/vcr_cassettes/manageiq/providers/azure/cloud_manager/refresher_targeted_scope/lb_with_vms_refresh.yml
spec/vcr_cassettes/manageiq/providers/azure/cloud_manager/refresher_targeted_scope/lbs_refresh.yml
spec/vcr_cassettes/manageiq/providers/azure/cloud_manager/refresher_targeted_scope/multiple_targets_refresh.yml
spec/vcr_cassettes/manageiq/providers/azure/cloud_manager/refresher_targeted_scope/orchestration_stack_refresh.yml
spec/vcr_cassettes/manageiq/providers/azure/cloud_manager/refresher_targeted_scope/orchestration_stack_vm_refresh.yml
spec/vcr_cassettes/manageiq/providers/azure/cloud_manager/refresher_targeted_scope/powered_off_vm_refresh.yml
spec/vcr_cassettes/manageiq/providers/azure/cloud_manager/refresher_targeted_scope/powered_on_vm_refresh.yml
spec/vcr_cassettes/manageiq/providers/azure/cloud_manager/refresher_targeted_scope/vm_with_managed_disk_refresh.yml
|
|
Checked commits https://github.com/djberg96/manageiq-providers-azure/compare/7c0f52585e5f2d3b2d7a02ee94452362ad71436d~...9f93c661d4abf55d1dd72cedd2dcaa78cac18a29 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 spec/models/manageiq/providers/azure/cloud_manager/targeted_refresher_scope_spec.rb
|
|
@Ladas Not sure why, but another targeted refresh spec is failing after regenerating the cassettes, so I commented out the offending spec for now. Something about the vm from an orchestration template it doesn't like, but I'm not sure what. Other than that, it's ready to go. |
| # strings using the config/settings.yml from the provider repo. | ||
|
|
||
| def cached_resource_provider_service(config) | ||
| @cached_resource_provider_service ||= resource_provider_service(config) |
There was a problem hiding this comment.
if this is something that rarely changes, we could probably wrap it to cache_with_timeout block?
There was a problem hiding this comment.
I didn't know about cache_with_timeout but perhaps we can switch it to that if you prefer.
There was a problem hiding this comment.
yeah, for the things that do not change (or not frequently) it's perfect
If a user sets an invalid api-version string in the config/settings.yml file, then the refresh parser will bomb with an error from Azure. This could happen because they modified it manually, or because their environment has different values that are considered valid from what we specify by default.
This PR handles that possibility gracefully by checking the api-version specified in the settings.yml file versus the list of possible values that the Azure REST API tells us are valid. If there's a match, then things proceed normally.
If not, however, then we pull the most recent api-version string from the list of valid values and use that instead, logging a warning for good measure.
WIP until I can add some specs.