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

docs: updated sqs binding readme file #214

Merged
merged 1 commit into from
Sep 18, 2023

Conversation

gokerakc
Copy link
Contributor

@gokerakc gokerakc commented Sep 4, 2023

Description

  • type: standard is not a valid property. Based on the SQS schema we need to use the property fifoQueue.

More details

In the SQS binding schema, we are using the fifoQueue property (also you can see in this table) to define the queue whether is a standard or a FIFO one.

But in the SQS readme example, type: standard was used and this does not match with the schema itself. So to fix that mismatching we need to use fifoQueue: false instead of type: standard in the readme example.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

@derberg
Copy link
Member

derberg commented Sep 4, 2023

@dpwdec @iancooper please have a look

Copy link
Collaborator

@iancooper iancooper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AWS supports two queue types for SQS: standard and FIFO. Please see: https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/sqs-queue-types.html

Is there a problem underlying this request?

//cc @dpwdec

@gokerakc
Copy link
Contributor Author

gokerakc commented Sep 4, 2023

I've edited the PR description to explain the issue better.

@dpwdec
Copy link
Collaborator

dpwdec commented Sep 4, 2023

@iancooper we changed the schema to reflect the AWS API for making a queue FIFO or Standard which is done via a boolean called FifoQueue. In the AsyncAPI schema here

I believe @gokerakc ‘s PR is just bringing the examples in line with the schema because these were missed on the initial commit for the SQS binding and still show the original design.

@iancooper
Copy link
Collaborator

Thanks for that @gokerakc and @dpwdec. Type only really has advantages if AWS issues another type, so that we cannot use a boolean flag. That is however speculative, and given that AWS would have to adjust their API to fix the issue, I think we can agree to opt for what would be most obvious for an AWS developer, in this case using the boolean flag instead of a type field.

Thanks for the explanation.

@derberg
Copy link
Member

derberg commented Sep 13, 2023

@iancooper @dpwdec I guess this one is ok to be merged?

@dpwdec dpwdec merged commit 85b0037 into asyncapi:master Sep 18, 2023
23 checks passed
@derberg
Copy link
Member

derberg commented Sep 25, 2023

@dpwdec fyi, next time you can also merge with /rtm comment under a PR. This way you are sure that merging is done 100% accurate with proper PR. Most important, less clicking 😄

@gokerakc gokerakc deleted the update-readme-files branch February 13, 2024 14:15
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.

4 participants