Repository navigation
Conversation
JackKelly-Bellroy
left a comment
There was a problem hiding this comment.
Beware copying from Amazonka too closely, it's constrained by what botocore says about data types when its generator emits the Haskell record definitions. We had to write amazonka-dynamodb-attributevalue to provide an actual sum type to make DynamoDB more ergonomic to use. Would it be worth doing similar here?
| binaryValue :: Maybe ByteString, | ||
| stringListValues :: [Text], | ||
| binaryListValues :: [ByteString], | ||
| dataType :: Text |
There was a problem hiding this comment.
Is this actually a sum type and should it be modeled as such?
There was a problem hiding this comment.
That's a good point, but dataType is slightly more complex than a closed enum. It can be String, Number, Binary, or one of the three plus .custom-label, for example String.hello-world. We could have something like:
data LabeledMessageAttributeValue = LableledMessageAttributeValue
{ dataTypeLabel :: Text,
value :: MessageAttributeValue
}
data MessageAttributeValue = String Text | Binary ByteString | Number ScientificDo you have any suggestion about how to name these things?
There was a problem hiding this comment.
From a discussion @kokobd and I had over voice: we decided on the call it's probably best to keep this mapping as close to what SQS invokes, so library users can see how to use it. But now, looking at the snippet you've posted above, I think something like that would be quite workable, perhaps with customTypeLabel :: Maybe Text:
data MessageAttribute = MessageAttribute
{ customTypeLabel :: Maybe Text
, value :: MessageAttributeValue
}
data MessageAttributeValue
= Binary ByteString
| Number Text
| String TextThere was a problem hiding this comment.
I agree that we should make types ADT/Haskell-like where they fit. It's unfortunate that this is so often made so difficult :)
From here:
Number – Number attributes can store positive or negative numerical values. A number can have up to 38 digits of precision, and it can be between 10^-128 and 10^+126.
So... kind of odd. More precision than an f64, less exponent range than an f128.
Since we can't match the domain exactly, we'll need to make it larger. I think my lean would be to use a Number128 or a Scientific. Both are strictly larger, but not quite so large as Text. Other than that, this last recommendation looks good to me, and I'm not too attached to any specific number type approach.
|
Ideally, I think we'll want to get this into 1.2 as well, since that will bundle the breaking changes in this area. Also, I think our best example of a safe default is the Rust SDK: https://docs.rs/aws_lambda_events/latest/aws_lambda_events/event/sqs/struct.SqsEvent.html since that's actually owned by AWS. There should be no need to type any more loosely than this. We may be able to type more strictly, but we'll want to do so carefully. |
1021f16 to
fbff269
Compare
fbff269 to
45c2434
Compare
SQS message attributes arrive as structured JSON objects, so
Map Text Textrejects events containing them. Decode attributes intoMessageAttribute, retaining custom type labels and representing values asString Text,Number Scientific, orBinary ByteString.The decoder selects the value field required by
dataType, preserves numeric precision and custom suffixes, and rejects unknown types or missing values. The reserved list fields are ignored. Standard queue events without a message group ID and events with empty message attributes remain supported.This changes the public SQS attribute type and is intended for 1.2. Bellroy's monorepo still uses its existing fork pin; adopting this API there requires a separate caller migration.