validate attributes when creating SQS queues #10820
Merged
+332
−22
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
cloutierMat
added
aws:sqs
cloutierMat
changed the title
LocalStack Community integration with Pro 2 files ± 0 2 suites ±0 1h 38m 11s ⏱️ + 1m 38s Results for commit d1d34d1. ± Comparison against base commit 3c4c463. This pull request removes 12 and adds 25 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
cloutierMat
requested review from
dominikschubert,
pinzon,
simonrw and
Morijarti
as code owners
simonrw
approved these changes
May 15, 2024
Comment on lines
1
to
2
| Conditions: | ||
| IsFifo: !Equals [ {{ is_fifo }}, "true"] |
There was a problem hiding this comment.
minor: this could be a CloudFormation parameter, rather than relying on our string interpolation:
Suggested change
| Conditions: | |
| IsFifo: !Equals [ {{ is_fifo }}, "true"] | |
| Parameters: | |
| IsFifo: | |
| Type: String | |
| Conditions: | |
| IsFifo: !Equals [ !Ref IsFifo, "true"] |
then deploy with
result = deploy_cfn_template(
template_path=os.path.join(
os.path.dirname(__file__), "../../../templates/sqs_fifo_autogenerate_name.yaml"
),
parameters={"IsFifo": "false"},
max_wait=240,
)Annoyingly CFn parameters cannot be booleans 🤦
| @@ -33,8 +33,7 @@ def test_sqs_fifo_queue_generates_valid_name(deploy_cfn_template): | |||
| assert ".fifo" in result.outputs["FooQueueName"] | |||
|
|
|||
|
|
|||
| # FIXME: doesn't work on AWS. (known bug in cloudformation: https://github.com/aws-cloudformation/cloudformation-coverage-roadmap/issues/165) | |||
| @markers.aws.unknown | |||
| @markers.aws.validated | |||
baermat
approved these changes
May 16, 2024
| valid = [ | ||
| k[1] | ||
| for k in inspect.getmembers( | ||
| QueueAttributeName, lambda x: isinstance(x, str) and not x.startswith("__") |
There was a problem hiding this comment.
purely curious, what's the reasoning behind the startswith clause, which attributes are weeded out here?
There was a problem hiding this comment.
get_members returns all of the members and dunder methods, after filtering for str, I believe it is __name__ that was left.
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.
Motivation
Looking into issue #10812, I realised that the attributes were not being validated at queue creation.
While the documentation does mention
falseas an appropriate value for this attribute, reports are dating back 2017 that providing it results inAttributeNameError.Changes
The method
validate_queue_attributeswas already enforcing this behaviour but it wasn't called when originally creating the queue.A test for fifo queue attributes is also now fixed. 😄
fix #10812