Added SNS & SQS Events #46
Conversation
fabianfett
force-pushed
the
sns-and-sqs-event
branch
from
5ea74b3 to
792189b
Compare
| self = .string(value) | ||
| case "Number": | ||
| let value = try container.decode(AWSNumber.self, forKey: .stringValue) | ||
| self = .number(value) |
There was a problem hiding this comment.
should .number just be the numeric value instead of the AWSNumber decoding envelope?
There was a problem hiding this comment.
I hadn't thought about this so far. Tried it today and ran into two issues:
- Protocol 'Numeric' can only be used as a generic constraint because it has Self or associated type requirements.
public enum Attribute {
case string(String)
case binary([UInt8])
case number(Numeric)
}
I don't think we should make Attribute generic.
- Even if this was working, how would we parse the number in this case? Try to parse into Int and if that doesn't work parse into Double?
There was a problem hiding this comment.
i was thinking the enum option will include int and double instead of generic number. wdyt?
There was a problem hiding this comment.
That sounds reasonable, but we would not use the original data format which by itself is just a plain String. Anyway, how would we parse this? Try Int first? If this doesn't work try to parse Double? Your call.
There was a problem hiding this comment.
@tomerd do you have any input on this?
There was a problem hiding this comment.
to be clear, I was not pushing against using AWSNumber for decoding, but against "leaking" it in the enum. I guess you are asking if we should first try AWSNumber's double accessor or the int accessor first to determine how to populate the enum?
There was a problem hiding this comment.
Okay. I can follow you in not wanting to not the leak the AWSNumber type. Do you have an idea in mind how you would like to achieve this?
Package.swift
Outdated
| .target(name: "AWSLambdaEvents", dependencies: []), | ||
| .target(name: "AWSLambdaEvents", dependencies: [ | ||
| .product(name: "NIOHTTP1", package: "swift-nio"), | ||
| .product(name: "NIOFoundationCompat", package: "swift-nio"), |
There was a problem hiding this comment.
Upps. Forgot to remove when I was cherry picking. Fixed.
| // of four. | ||
| try Base64.decode(encoded: self, options: options) | ||
| } | ||
| } |
There was a problem hiding this comment.
are we missing the tests for the base64 utility?
fabianfett
force-pushed
the
sns-and-sqs-event
branch
from
4c26e48 to
287f235
Compare
fabianfett
force-pushed
the
sns-and-sqs-event
branch
from
287f235 to
1e35d1d
Compare
Sources/AWSLambdaEvents/SQS.swift
Outdated
| self.awsRegion = try container.decode(AWSRegion.self, forKey: .awsRegion) | ||
|
|
||
| let body = try container.decode(String?.self, forKey: .body) | ||
| self.body = body != "" ? body : nil |
There was a problem hiding this comment.
is this required? ie synthesized decoding not working?
No description provided.