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

The :geo_location attribute needs to be xml formatted before calling aws #142

Merged
merged 2 commits into from
Jun 29, 2015

Conversation

carloslima
Copy link
Contributor

Without this, calling record.destroy on a record that has geolocation set will cause an exception with:

Fog::DNS::AWS::Error: MalformedInput => Unexpected complex element termination

Without this, calling `record.destroy` on a record that has geolocation
set will cause an exception with:
Fog::DNS::AWS::Error: MalformedInput => Unexpected complex element termination
@carloslima
Copy link
Contributor Author

I moved the request body generation out of the change_resource_record_sets method so that I could test it separately.
I'm not very familiar on how the tests should be structured for this project; let me know if that was a mistake :-)

Is there a need to validate the attribute before turning to xml? If so, should it happen in change_resource_record_sets_data or somewhere else?
Or, if anything else is not good, let me know and I'd be more than happy to fix.

Cheers!

@carloslima
Copy link
Contributor Author

The diff on github's web interface looks a lot more messier than it actually is.
The no-whitespace version looks a lot more clear: https://github.com/fog/fog-aws/pull/142/files?w=1

:-)

@lanej
Copy link
Member

lanej commented Jun 29, 2015

@carloslima non-whitespace is much better, thanks.

lanej added a commit that referenced this pull request Jun 29, 2015
…uest

The :geo_location attribute needs to be xml formatted before calling aws
@lanej lanej merged commit 80ca9fd into fog:master Jun 29, 2015
@lanej
Copy link
Member

lanej commented Jun 29, 2015

@carloslima there doesn't appear to be a great way to test request body transformation atm. @fog/fog-aws might have some better input.

@carloslima
Copy link
Contributor Author

Thanks! 😃

@geemus
Copy link
Member

geemus commented Jun 30, 2015

Yeah, afraid we haven't really tested this explicitly in the past. Not sure how best to add it in now.

@geemus
Copy link
Member

geemus commented Jun 30, 2015

Thanks!

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.

3 participants