swift-server / swift-aws-lambda-runtime

Swift implementation of AWS Lambda Runtime
https://swiftpackageindex.com/swift-server/swift-aws-lambda-runtime
Apache License 2.0
1.15k stars 104 forks source link

Remove requirement for macOS 10.13 #152

Closed helje5 closed 4 years ago

helje5 commented 4 years ago

The runtime requires macOS 10.13, which is quite inconvenient because all consuming packages now also need to explicitly require 10.13 in the package manifest. It is especially annoying if one tries to conditionally implement Lambda support in a 10.10 package via #canImport, e.g.: https://github.com/Macro-swift/Macro/blob/feature/lambda-1/Sources/http/Lambda/lambda.swift#L9

Slack says this requirement only exists for the ISO date formatter, not sure whether you use it for rendering only or also for parsing. In any case, it should be quite easy to replace w/ a custom parser/renderer. Should be reasonable because the format is fixed and never changes.

It could be built on top of timegm and strptime, though it needs an extra processing step to capture the milliseconds:

import func   Foundation.strptime
import func   Foundation.timegm
import struct Foundation.tm

let s          = "20180905T140903.591680Z"
var parsedTime = tm()

s.withCString { cstr in
  strptime(cstr, "%Y%m%dT%H%M%S.%fZ", &parsedTime) // this needs to extract the ms
}
let time = timegm(&parsedTime)
let ti   = TimeInterval(time) // + milliseconds

You could also provide this, and still use the ISO formatter if available (via if #available(macOS 10.13, ...)), which is what I do in Macro.

fabianfett commented 4 years ago

I'm a +1 on this and think that we should address it. @tomerd wdyt?

tomerd commented 4 years ago

+1 conceptually - I dont like the 10.13 constraint either.

do we know if this can be done in all formats coming form AWS? I think foundation used ICU for some timezone localization aspects which may be more complicated than the one demonstrated here, but it may not be used here

helje5 commented 4 years ago

? It’s ISO, there are only really two variants, with ms and without

helje5 commented 4 years ago

Oh, timezones, this format doesn’t support timezones (anything locale bound requiring tzdata) , max gmt offsets, though it probably is Z always

tomerd commented 4 years ago

right, but IIRC there are other date formats we deal with in the library (unfortunately different AWS events have different date formats) so trying to understand if we can get away with this in all cases

helje5 commented 4 years ago

neither dateformatter nor locale require 10.13, it is just the specific isodateformatter, and iso is fixed

tomerd commented 4 years ago

works for me!

fabianfett commented 4 years ago

@tomerd We can go full strptime here or we just create a DateFormatter that has the iso as the dateFormat string like here:

    let formatter = DateFormatter()
    formatter.dateFormat = "yyyy-MM-dd'T'HH:mm:ss.SSSZ"
    formatter.timeZone   = TimeZone(secondsFromGMT: 0)
    formatter.locale     = Locale(identifier: "en_US_POSIX")
    return formatter

https://github.com/fabianfett/swift-lambda-runtime/blob/c306ac8b32a79a42ef8ada855a79930f3d368a2a/Sources/LambdaEvents/SNS.swift#L94

helje5 commented 4 years ago

date formatters are really really slow, i kicked them out of shrugs because they have been taking like 40% of the startup time. they are locale aware which is zarro necessary here

helje5 commented 4 years ago

i think strptime is fine, but the ms need to be handled separately

tomerd commented 4 years ago

ugh, looks like strptime not avail on through Glibc. I guess I could make this Darwin only since we will use the formatters in linux anyways