Allow SQS Visibility Timeout to be an option #9
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
To allow the consumer from SQS to potentially override the visibility timeout set on the queue, each message read from the queue can set it upon the read action.
|
Hi @dokie! Nice work! Can you please run Thanks! |
|
Hi I have ran it on my local machine and I always have the formatter run on file saves too so it seems not to change anything. I will try down versioning my Elixir version from 1.8 to 1.7 and see if that helps. Mike |
josevalim
reviewed
Apr 5, 2019
lib/broadway_sqs/ex_aws_client.ex
Outdated
| do: validation_error(:max_number_of_messages, "a integer between 1 and 10", value) | ||
| defp validate_option(:max_number_of_messages, value) | ||
| when value not in 1..@max_num_messages_allowed_by_aws, | ||
| do: |
There was a problem hiding this comment.
Given those are large, let's make them proper do/end blocks.
josevalim
reviewed
Apr 5, 2019
lib/broadway_sqs/ex_aws_client.ex
Outdated
|
|
||
| defp validate_option(:visibility_timeout, value) | ||
| when value not in 0..@max_visibility_timeout_allowed_by_aws_in_seconds, | ||
| do: |
There was a problem hiding this comment.
Given those are large, let's make them proper do/end blocks.
|
Also, should we update the docs? @msaraiva can you point to the proper place where that would be? |
Also added ASDF files to .gitignore
|
Looks good. Documentation has been already updated. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
To allow the consumer from SQS to potentially override the visibility timeout set on the queue, each message read from the queue can set it upon the read action.
I ultimately am looking at how additionally I could set this on each read at runtime to cater for a slow running consumer but without setting a very high value on the queue or in this option. This will allow a kind of "keep-alive" for a message being worked upon. This is facilitated via this call on the AWS API and I think this is supported on the
ExAws.SQSclient.