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

Adds nullish coalescing to output of Aws::SQS::Client#receive_message to an empty array #753

Merged
merged 1 commit into from
Nov 23, 2023

Conversation

mkeemon
Copy link
Contributor

@mkeemon mkeemon commented Nov 21, 2023

Fixes #752 due to change in behavior of aws-sdk-sqs (aws/aws-sdk-ruby#2947)

  • This adds a default value of [] to all invocations of Aws::SQS::Client#receive_message
  • The underlying behavior in the AWS Ruby SDK appears to have been patched, but this will prevent regressions in the future should something similar happen again

@mkeemon
Copy link
Contributor Author

mkeemon commented Nov 21, 2023

@phstc any ideas on why moto and Aws::SQS::Client are now at odds? Moto is returning XML while the client expects JSON. Did you run into any similar issues when creating the newest release a few weeks ago?

Moto

  • Python version: 3.12.0
  • Moto Version: 4.2.9

AWS Client

  • aws-sdk-core: 3.187.1
  • aws-sdk-sqs: 1.67.0
Failure/Error: Shoryuken::Client.sqs.delete_queue(queue_url: queue_url)
     Aws::Json::ParseError:
       unexpected token at '<DeleteQueueResponse><ResponseMetadata><RequestId>qYSGBzaVm7ABwMZ9tT15MD8XDLycSkezkGhzDSFKhCmgCc68O3re</RequestId></ResponseMetadata></DeleteQueueResponse>'
$ pip show moto
Name: moto
Version: 4.2.9
Summary: 
Home-page: https://github.com/getmoto/moto
Author: Steve Pulec
Author-email: "spulec@gmail.com"
License: Apache License 2.0
Location: /usr/local/lib/python3.12/site-packages
Requires: boto3, botocore, cryptography, Jinja2, python-dateutil, requests, responses, werkzeug, xmltodict

@phstc
Copy link
Collaborator

phstc commented Nov 21, 2023

@mkeemon thanks for checking it. The last release passed ✅ fine #751

image

I'm wondering if there's something with the AWS change, and now we need to specify the type to be JSON instead of XML somewhere here.

I couldn't find an option on the initializer to configure that though.

@mkeemon
Copy link
Contributor Author

mkeemon commented Nov 22, 2023

What if I am already on the latest AWS SDK version, but my open sourced solution does not support JSON?
You must change your SDK version to the version previous to what you are using. See How do I get started with AWS JSON protocols for Amazon SQS? for more infomation. AWS SDK versions listed in How do I get started with AWS JSON protocols for Amazon SQS? uses JSON wire protocol for Amazon SQS APIs. If you change your AWS SDK to the previous version, your Amazon SQS APIs will use the AWS query.

https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/sqs-json-faqs.html

It looks like 1.67.0 of aws-sdk-sqs, which came out a couple weeks ago, moves to JSON. I'll try pinning the version to 1.66.0 1.65.0 for now and open up an issue in Moto repo to migrate the responses to JSON.

Edit: 1.66.0 introduces JSON - 1.65.0 works with XML + Moto

@phstc
Copy link
Collaborator

phstc commented Nov 22, 2023

@mkeemon great find! Re-running tests. Thanks for helping with this. I really appreciate it.

Gemfile Show resolved Hide resolved
… to an empty array

Pins aws-sdk-sqs to 1.65.0 to allow for XML responses
@mkeemon mkeemon force-pushed the fix-sqs-client-receive_message branch from 18cffbb to 24640c0 Compare November 22, 2023 22:07
@mkeemon
Copy link
Contributor Author

mkeemon commented Nov 22, 2023

@phstc done and done!

@phstc phstc enabled auto-merge (squash) November 23, 2023 04:10
@phstc phstc merged commit c966255 into ruby-shoryuken:main Nov 23, 2023
19 checks passed
@mkeemon mkeemon deleted the fix-sqs-client-receive_message branch November 23, 2023 12:33
@mscrivo
Copy link

mscrivo commented Nov 23, 2023

I'm a bit curious, why the need to pin the version of aws sdk for tests if this PR fixes the issue with the newer versions? Won't that just prevent finding issues with newer versions of the sdk in the future?

@mkeemon
Copy link
Contributor Author

mkeemon commented Nov 24, 2023

@mscrivo This PR was merged slightly before the one in Moto. I didn't realize just how quickly the Moto team would take action on this. We should be able to remove the strict versioning in the Gemfile now.

Edit: It doesn't look like Moto has cut a new release that contains these changes just yet.

@phstc
Copy link
Collaborator

phstc commented Nov 28, 2023

@mkeemon version 6.1.1 is out with this PR! Thank you 🎉

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.

Exception in Shoryuken::Queue#receive_messages
3 participants