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

Allowing for order! to be a string key #44

Merged
merged 1 commit into from
Aug 27, 2014

Conversation

hugocorbucci
Copy link

When using savon to issue SOAP requests in a background process, the
request parameters (a hash) get saved by resque in redis. When pulled
out of redis, all keys of the hash are stringified as they are
marshalled as strings.
In order to avoid ordering issues when using resque or sidekick, gyoku
will accept both the symbol or the string version of the order! key and
respect that order.
This way savon can take whatever comes from those backgrounding tools and convert it to XML without having to worry about deeply reconverting everything to symbols.

When using savon to issue SOAP requests in a background process, the
request parameters (a hash) get saved by resque in redis. When pulled
out of redis, all keys of the hash are stringified as they are
marshalled as strings.
In order to avoid ordering issues when using resque or sidekick, gyoku
will accept both the symbol or the string version of the order! key and
respect that order.
@tjarratt
Copy link
Contributor

Thanks for the pull request @hugocorbucci. I'm sorry it took me so many weeks to respond to this issue -- I certainly didn't want to keep you waiting for a response this long.

This issue is a bit of an edge case, but it seems like a perfectly fine change that has a very low risk of ever hurting anyone but... it does make the public API a little bit inconsistent.

tjarratt added a commit that referenced this pull request Aug 27, 2014
Allowing for order! to be a string key
@tjarratt tjarratt merged commit 9083365 into savonrb:master Aug 27, 2014
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.

2 participants