-
-
Notifications
You must be signed in to change notification settings - Fork 271
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
Locking Savon to < 2.13 to prevent an error making queries #558
Conversation
@avanrielly great! Going to merge, but can you submit another PR with a changelog entry? |
@iloveitaly Sorry, I did not even realize this was part of the process. I have made a follow-up PR here |
We may want to revert this (and #560) as the underlying issue in Savon was fixed and released as v2.13.1, and I confirmed the new version works correctly for me: v2.13.0 will continue to be broken, but restricting anyone from using the newer v2.13.1 (or future versions) doesn't seem ideal. |
@cgunther great idea! I think we can ban specific versions using |
This version of Savon mistakenly prefixed the default namespace of `xmlns`, resulting in a namespace of `xmlns:xmlns`, which caused NetSuite to error on requests. v2.13.1 fixed this issue to skip prefixing the default namespace. This is a continuation of NetSweet#558, which avoided v2.13.0 and anything greater. Bug: savonrb/savon#943 (comment) Fix: savonrb/savon#977
This version of Savon mistakenly prefixed the default namespace of `xmlns`, resulting in a namespace of `xmlns:xmlns`, which caused NetSuite to error on requests. v2.13.1 fixed this issue to skip prefixing the default namespace. This is a continuation of NetSweet#558, which avoided v2.13.0 and anything greater. Bug: savonrb/savon#943 (comment) Fix: savonrb/savon#977
…563) * Avoid Savon version `2.13.0` to prevent generating invalid envelopes This version of Savon mistakenly prefixed the default namespace of `xmlns`, resulting in a namespace of `xmlns:xmlns`, which caused NetSuite to error on requests. v2.13.1 fixed this issue to skip prefixing the default namespace. This is a continuation of #558, which avoided v2.13.0 and anything greater. Bug: savonrb/savon#943 (comment) Fix: savonrb/savon#977 * Use RC of mail gem in CI for Ruby 3.1 support Savon v2.13 adds a dependency on mail, which has an implicit dependency on net-smtp. Ruby 3.1 removed net-smtp as a default gem. mail v2.8.0.rc1 is the first to make that dependency explicit to support Ruby 3.1. This at least gets CI passing, but may not help end-users on Ruby 3.1. If they previously had a dependency on mail (ie. through Rails), there's probably already an explicit dependency in their app on net-smtp to get it working on Ruby 3.1. If they have no dependency on mail previously, this will likely error as they go to require this gem, which requires savon, which requires mail, which requires net-smtp, but Ruby can't find it. It doesn't seem right to add net-smtp (or an RC of mail) to this gem's gemspec dependencies, given we don't directly use either library. Ultimately it's about the latest stable release of mail being incompatible with Ruby 3.1.
Making a request with the NetSuite gem and Savon 2.13.0 caused the following error:
Therefore, this PR locks the Savon gem to the last versions that work.
If I had to guess at the PR that caused the issue, I would suggest it is this one.