Skip to content

Conversation

@nalundgaard
Copy link
Contributor

@nalundgaard nalundgaard commented Jan 5, 2022

Follow-up to #713.

When AWS added SqsManagedSseEnabled, we were lucky that it was a Boolean value, so it didn't break erlcloud_sqs:decode_attribute_value/1, which assumes that all attribute values that aren't specifically known are Boolean or integer values. This change anticipates a future when they add a string attribute value (like QueueArn or KmsMasterKeyId ), so that we don't crash decoding THAT.

Finally, adding some get_queue_attributes testing to erlcloud_sqs_tests to cover this functionality.

Edit: also added these enhancements to get tests to pass:

erlcloud_sqs: extend sqs_queue_attribute_names

  • Reorder encode_attribute_name and decode_attribute_name to use the ordering from GetQueueAttributes API Reference AttributeName.N definition (alphabetical, with common params first, then SSE & FIFO specific attribute names in sections below).
  • Add FifoQueue and ContentBasedDeduplication to encode_attribute_name and decode_attribute_name, previously missed FIFO parameters that should be supported.
  • Refactor sqs_queue_attribute_name type to include all known types.

Follow-up to #713.

When AWS added `SqsManagedSseEnabled`, we were lucky that it was a Boolean value, so it didn't break `erlcloud_sqs:decode_attribute_value/1`, which assumes that all attribute values that aren't specifically known are Boolean or integer values. This change anticipates a future when they add a string attribute value (like `QueueArn` or `KmsMasterKeyId `), so that we don't crash decoding THAT.

Finally, adding some `get_queue_attributes` testing to `erlcloud_sqs_tests` to cover this functionality.
@nalundgaard nalundgaard requested a review from motobob January 5, 2022 02:37
@nalundgaard
Copy link
Contributor Author

cc @neoascetic @cody-friedrichsen

* Reorder `encode_attribute_name` and `decode_attribute_name` to use the ordering from [GetQueueAttributes API Reference](https://docs.aws.amazon.com/AWSSimpleQueueService/latest/APIReference/API_GetQueueAttributes.html) `AttributeName.N` definition (alphabetical, with common params first, then SSE & FIFO specific attribute names in sections below).
* Add `FifoQueue` and `ContentBasedDeduplication` to `encode_attribute_name` and `decode_attribute_name`, previously missed FIFO parameters that should be supported.
* Refactor `sqs_queue_attribute_name` type to include all known types.
@nalundgaard
Copy link
Contributor Author

Note that I added a new commit to cover a couple more issues:

erlcloud_sqs: extend sqs_queue_attribute_names

  • Reorder encode_attribute_name and decode_attribute_name to use the ordering from GetQueueAttributes API > Reference AttributeName.N definition (alphabetical, with common params first, then SSE & FIFO specific attribute names in sections below).
  • Add FifoQueue and ContentBasedDeduplication to encode_attribute_name and decode_attribute_name, previously missed FIFO parameters that should be supported.
  • Refactor sqs_queue_attribute_name type to include all known types.

The sqs_queue_attribute_name type spec was what made dialyzer angry in OTP 24, because someone had missed delay_seconds and receive_message_wait_time_seconds in the spec, and I was passing those into my argument. Lovely. Hopefully all G2G now.

@nalundgaard nalundgaard merged commit e6b5bd6 into master Jan 5, 2022
@nalundgaard nalundgaard deleted the sqs_convert_integer_fix branch January 5, 2022 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants