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

Improve AWS SQS Sensor #16880

Closed
baolsen opened this issue Jul 8, 2021 · 5 comments · Fixed by #16904
Closed

Improve AWS SQS Sensor #16880

baolsen opened this issue Jul 8, 2021 · 5 comments · Fixed by #16904
Labels
kind:feature Feature Requests provider:amazon-aws AWS/Amazon - related issues

Comments

@baolsen
Copy link
Contributor

baolsen commented Jul 8, 2021

Description

Improve the AWS SQS Sensor as follows:

  • Add optional visibility_timeout parameter
  • Add a customisable / overrideable filter capability so we can filter/ignore irrelevant messages

[Not needed, see below conversation]
--- Check the HTTP status code in AWS response and raise Exception if not 200 - best practice

Use case / motivation

I'd like to make the SQS sensor more flexible to enable the following use case:

  • A single queue can be used as a channel for messages from multiple event sources and or multiple targets
  • We need a way to filter and ignore messages not relevant to us, which other processes are looking for

Are you willing to submit a PR?

Yes, please assign to me

Related Issues

None

@baolsen baolsen added the kind:feature Feature Requests label Jul 8, 2021
@ashb
Copy link
Member

ashb commented Jul 8, 2021

Check the HTTP status code in AWS response and raise Exception if not 200 - best practice

Doesn't Boto do that for us already?

@baolsen
Copy link
Contributor Author

baolsen commented Jul 8, 2021

Check the HTTP status code in AWS response and raise Exception if not 200 - best practice

Doesn't Boto do that for us already?

Yea I have a gripe about the usability/accuracy of the AWS documentation sometimes.

Some of the boto3 APIs do indeed provide a managed capability so we don't need to check return codes.
But many (most?) others do not, especially older ones. It's not always clear in the documentation which is which... and there are no "since" tags on the APIs either.
I have noticed that if an API is managed it is usually called out as such, but even then some managed APIs don't fully isolate the user from certain issues ;)

For the SQS receive_message API in particular -

The boto3 response does not document explicitly that it returns ResponseMetadata, but in practice it definitely does return it :)
https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/sqs.html#SQS.Client.receive_message

At the top of the boto3 SQS page, client section, additional information section, they do link to this:
https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/sqs-api-responses.html

So you kind of have to stumble upon it... but that is the clearest reference I could find.

There is also this guide:
https://docs.aws.amazon.com/AWSSimpleQueueService/latest/APIReference/API_ReceiveMessage.html
Which refers to "Common errors" but is not very clear on where to detect the errors.

This developer guide link also doesn't clarify, in fact it doesn't mention the response metadata at all:
https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/working-with-messages.html#handling-request-errors

In my SQS responses I do see the ResponseMetadata element that can be (and presumably should be) checked.

So the check I would like to add is the usual:

if not response['ResponseMetadata']['HTTPStatusCode'] == 200:
    raise Exception('An HTTP error occurred')

@baolsen
Copy link
Contributor Author

baolsen commented Jul 8, 2021

Edit: The "clearest reference" link doesn't even mention checking the HTTP code which is definitely in the responses I have.... So yea not clear at all then ;)

@ashb
Copy link
Member

ashb commented Jul 8, 2021

@baolsen
Copy link
Contributor Author

baolsen commented Jul 9, 2021

That is awesome, thank you for sharing. I'll scratch that one from this request :)

@eladkal eladkal added the provider:amazon-aws AWS/Amazon - related issues label Jul 9, 2021
potiuk pushed a commit that referenced this issue Aug 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:feature Feature Requests provider:amazon-aws AWS/Amazon - related issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants