Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change get_template to return a string #266

Merged
merged 1 commit into from Mar 23, 2017
Merged

Conversation

djberg96
Copy link
Collaborator

@djberg96 djberg96 commented Mar 23, 2017

The TemplateDeploymentService#get_template method currently tries to generate and return a DeploymentTemplate model object. However, this is problematic because a template can contain arbitrary text that our base model cannot translate into a real Ruby constant.

Consequently, this method has been altered to just return a plain JSON string instead of a model object. While this technically breaks backwards compatibility, the reality is that this method is likely unused in the wild since it's just a matter of time before it fails, and that the raw JSON is what people are after anyway. ~~~That said, I have left the DeploymentTemplate model in the code for those people who may want to try and convert it to an object on their own.~~~

Addresses https://bugzilla.redhat.com/show_bug.cgi?id=1434988

@djberg96 djberg96 requested a review from bzwei March 23, 2017 20:36
#
def get_template(deploy_name, resource_group = configuration.resource_group)
validate_resource_group(resource_group)
validate_resource(deploy_name)
url = build_url(resource_group, deploy_name, 'exportTemplate')
response = JSON.parse(rest_post(url))['template']
DeploymentTemplate.new(response)
JSON.parse(rest_post(url))['template'].to_json
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should remove DeploymentTemplate from base_model.rb.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bzwei I thought about it, but thought it might be nice for people who do want to convert it. Or do you think that will just lead to confusion?

Remove DeploymentTemplate model.
@djberg96
Copy link
Collaborator Author

@bzwei Ok, removed that model.

@miq-bot
Copy link
Member

miq-bot commented Mar 23, 2017

Checked commit djberg96@5f1735a with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks good. 🍰

@bzwei bzwei merged commit 9c20485 into ManageIQ:master Mar 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants