TemplateDeploymentService#get_template bug about azure-armrest HOT 9 CLOSED

I'm actually wondering now why we return a model object instead of the raw template there.

from azure-armrest.

@djberg96 Good point. This is the a new method added recently. Indeed we may not want to model it. Can you try to add it the exclude list in https://github.com/ManageIQ/azure-armrest/blob/master/lib/azure/armrest/model/base_model.rb#L211?

from azure-armrest.

Actually it is not the exclude list. https://github.com/ManageIQ/azure-armrest/blob/master/lib/azure/armrest/template_deployment_service.rb#L50 we should NOT model DeploymentTemplate.

from azure-armrest.

Perhaps it was originally a bad decision, but I'd like to keep backwards compatibility. Please see: #265

from azure-armrest.

@djberg96 although #265 addresses a potential issue for modeling, it does not solve the problem for deployment template. The template may not be deployed properly if the content has been changed.

We have to make the change here even if it breaks the backward compatibility.

from azure-armrest.

@bzwei Ah, true enough. Ok, I'll modify get_template as well then.

This shouldn't be a problem for ManageIQ since it's calling .to_s on it anyway.

from azure-armrest.

@bzwei Actually, that shouldn't modify the content, since the json is stored as its own attribute and __setobj__(@hash.dup) uses a dup.

from azure-armrest.

@djberg96 our current implementation of to_s is not correct because it does not pickup any changes through setter methods.

from azure-armrest.

Fixed by #266.

from azure-armrest.