search

soto-project / soto

Swift SDK for AWS that works on Linux, macOS and iOS
https://soto.codes
Apache License 2.0
878 stars 83 forks source link

AWSHTTPClient.shutdown doesn't state Sendable callback #621

Closed vojtarylko closed 2 years ago

vojtarylko commented 2 years ago

Current AWSHTTPClient.shutdown doesn't require Sendable callback:

func shutdown(queue: DispatchQueue, _ callback: @escaping (Error?) -> Void)

but corresponding function in HTTPClient does.

This fails the compilation with (on Swift 5.6)

soto-core/Sources/SotoCore/HTTP/AsyncHTTPClient.swift:21:1: error: type 'HTTPClient' does not conform to protocol 'AWSHTTPClient'
extension AsyncHTTPClient.HTTPClient: AWSHTTPClient {
^
async-http-client/Sources/AsyncHTTPClient/HTTPClient.swift:198:33: note: candidate has non-matching type '(DispatchQueue, @escaping @Sendable (Error?) -> Void) -> ()'

    @preconcurrency public func shutdown(

soto-core/Sources/SotoCore/HTTP/AWSHTTPClient.swift:57:10: note: protocol requires function 'shutdown(queue:_:)' with type '(DispatchQueue, @escaping (Error?) -> Void) -> ()'
    func shutdown(queue: DispatchQueue, _ callback: @escaping (Error?) -> Void)

or produces warning (on Swift 5.7)

sendability of function types in instance method 'shutdown(queue:_:)' does not match requirement in protocol 'AWSHTTPClient'
adam-fowler commented 2 years ago

What version of Soto are you using?

vojtarylko commented 2 years ago

I'm using 6.1.2.

adam-fowler commented 2 years ago

And what version of SotoCore are you using. Maybe you need to run a package update. I've been compiling with both 5.6 and 5.7 for ages without issue.

adam-fowler commented 2 years ago

The latest async-http-client set that callback to be Sendable, which is really a breaking change for swift 5.6 and shouldn't have been implemented. swift-log did the same thing as well. I think this is general teething problems with adopting Sendable. I'll fix it for Soto as well

adam-fowler commented 2 years ago

This should be fixed by https://github.com/soto-project/soto-core/pull/518 when it gets merged. I would like to get the NIOLock warnings fixed as well before doing a merge, but swift-nio missed one method, which has just been rectified. Once they include that in a release I can do a release for SotoCore

vojtarylko commented 2 years ago

@adam-fowler Thanks for the fix!

adam-fowler commented 2 years ago

Fixed in v6.1.3