deprecate NIOEventLoopGroupProvider (#2142)

[weissi]

Motivation:

NIOEventLoopGroupProvider.createNew was probably never a good idea because it creates shutdown issues for any library that uses it. Given that we now have singleton (#2471) EventLoopGroups, we can solve this issue by just not having event loop group providers.

Users can just use group: any EventLoopGroup and optionally group: any EventLoopGroup = MultiThreadedEventLoopGroup.singleton.

Modifications:

• deprecate NIOEventLoopGroupProvider

Result:

• Libraries becomes easier to write and maintain.
• Fixes ecosystem issue: EventLoopGroupProvider and the 'similar create or share ELG' pattern #2142

[weissi]

Must be merged after #2471

[adam-fowler]

group: any EventLoopGroup = MultiThreadedEventLoopGroup.singleton is not a great alternative as it is no use if NIOPosix is not available. Also it would be good if apple/swift-nio-transport-services#180 was also merged before this got merged.

Any library that supports both Posix and Network framework will probably end up having to implement a new version of NIOEventLoopGroupProvider which replaces createNew with useSingleton or something similar. It might be better if this type came from NIO though to provide consistency.

[glbrntt]

The deprecation path could be a bit painful here as it affects libraries and users. e.g. NIO can deprecate this and users are stuck with warnings until intermediate libraries update their APIs and do a release. Not sure if I'm overthinking this.

We could just deprecate the createNew case. End users can provide .shared(MultiThreadedEventLoopGroup.singleton) until libraries add new APIs which just take any EventLoopGroup.

[Lukasa]

I'm kinda with George here, I think it'd be better to do a multi-stage approach. Causing all user libraries to immediately get deprecation warnings without a cycle for maintainers to add new API surface seems like it's suboptimal.

[weissi]

We could just deprecate the createNew case. End users can provide .shared(MultiThreadedEventLoopGroup.singleton) until libraries add new APIs which just take any EventLoopGroup.

Is the suggestion that libraries like Hummingbird and friends would add their own HummingBird.defaultEventLoopGroup or something and migrate from groupProvider: EventLoopGroupProvider = .createNew to groupProvider: EventLoopGroupProvider = .shared(HummingBird.defaultEventLoopGroup)?

At the same time they could add a group: any EventLoopGroup? = nil.

That would work for me. WDYT? @Lukasa / @glbrntt & @adam-fowler

[FranzBusch]

IMO, hummingbird and co just taking an EL is not great due to the relation ship of EL to bootstrap. I think we should just create that swift-nio-universal-bootstrap repo at some point where we expose a common way to bootstrap a client/server/datagram with a configuration that allows the user to choose .automatic, .posix or .niots. With the introduction of swift-certificates we have a way around the TLS configuration problem since we can just accept a Certificate and then serialise and deserialise.

If we would have that hummingbird and friends would accept a UniversalServerBootstrapConfiguration which would also include the EL configuration.

[glbrntt]

We could just deprecate the createNew case. End users can provide .shared(MultiThreadedEventLoopGroup.singleton) until libraries add new APIs which just take any EventLoopGroup.

Is the suggestion that libraries like Hummingbird and friends would add their own HummingBird.defaultEventLoopGroup or something and migrate from groupProvider: EventLoopGroupProvider = .createNew to groupProvider: EventLoopGroupProvider = .shared(HummingBird.defaultEventLoopGroup)?

At the same time they could add a group: any EventLoopGroup? = nil.

That would work for me. WDYT? @Lukasa / @glbrntt & @adam-fowler

Using the .shared would be the intermediate step so that end-users can avoid warnings while libraries put out new APIs.

I think the group: any EventLoopGroup? = nil where nil is "choose something sensible for me" is probably the right end goal.

Why would libraries need to provide a defaultEventLoopGroup? Seems like just the nil case of above.

[weissi]

Why would libraries need to provide a defaultEventLoopGroup? Seems like just the nil case of above.

@glbrntt Once we deprecate .createNew, what should Hummingbird change its API too? https://github.com/hummingbird-project/hummingbird/blob/c5dd619b1cd0a2683f0dd3d4b7a0bea12485060b/Sources/Hummingbird/Application.swift#L68C1-L73C1

Hummingbird can't change to eventLoopGroupProvider: NIOEventLoopGroupProvider = .shared(MultiThreadedEventLoopGroup.singleton) because that'll switch off NIOTS support by default. And they can't use .createNew (because deprecated).

The only interim step that's not deprecating the whole API (which I was originally intending to do with the deprecation of NIOEventLoopGroupProvider) is if Hummingbird provided its own .defaultEventLoopGroup (which switches between posix & TS) and then goes for

eventLoopGroupProvider: NIOEventLoopGroupProvider = .shared(...defaultEventLoopGroup)

or am I missing smth?

Alongside that, HB probably wants to introduce a new eventLoopGroup: any EventLoopGroup? = nil API anyway.

[glbrntt]

Why would libraries need to provide a defaultEventLoopGroup? Seems like just the nil case of above.

@glbrntt Once we deprecate .createNew, what should Hummingbird change its API too? https://github.com/hummingbird-project/hummingbird/blob/c5dd619b1cd0a2683f0dd3d4b7a0bea12485060b/Sources/Hummingbird/Application.swift#L68C1-L73C1

Hummingbird can't change to eventLoopGroupProvider: NIOEventLoopGroupProvider = .shared(MultiThreadedEventLoopGroup.singleton) because that'll switch off NIOTS support by default. And they can't use .createNew (because deprecated).

The only interim step that's not deprecating the whole API (which I was originally intending to do with the deprecation of NIOEventLoopGroupProvider) is if Hummingbird provided its own .defaultEventLoopGroup (which switches between posix & TS) and then goes for

eventLoopGroupProvider: NIOEventLoopGroupProvider = .shared(...defaultEventLoopGroup)

or am I missing smth?

I think what you're missing is that the intermediate deprecation is to help end users, i.e. people using Hummingbird et. al., but not Hummingbird itself.

What we should try to avoid is:

1. NIO is released, deprecating the provider
2. User updates and has deprecation warnings but no way to avoid them
3. Hummingbird et. al. get updated to provide an alternative API
4. User can move to alternative API

We're fine if 2 and 3 are reversed but we can't control that, so I suggest we do the following:

1. NIO is released, deprecating the .createNew case
2. User updates and has deprecation warning but can either live with the warning or use an appropriate singleton in the .shared case
3. Hummingbird et. al. get updated to provide an alternative API and deprecate their API using the provider
4. User can move to new API

Alongside that, HB probably wants to introduce a new eventLoopGroup: any EventLoopGroup? = nil API anyway.

The new API should be this ^

[adam-fowler]

Once we deprecate .createNew, what should Hummingbird change its API too

I guess Hummingbird would deprecate the initialiser using NIOEventLoopGroupProvider and add a new version that took (any EventLoopGroup)?. Although the default parameters might complicate the issue.

[weissi]

group: any EventLoopGroup = MultiThreadedEventLoopGroup.singleton is not a great alternative as it is no use if NIOPosix is not available.

That's a good point.

Also it would be good if apple/swift-nio-transport-services#180 was also merged before this got merged.

Yes, I'll work on that rn.

Any library that supports both Posix and Network framework will probably end up having to implement a new version of NIOEventLoopGroupProvider which replaces createNew with useSingleton or something similar. It might be better if this type came from NIO though to provide consistency.

I'd hope that libraries could use eventLoopGroup: any EventLoopGroup? = nil where nil means "default" and what that means can be chosen by the library itself. That way we also don't leak the exact value into the API. WDYT?

[adam-fowler]

I'd hope that libraries could use eventLoopGroup: any EventLoopGroup? = nil where nil means "default" and what that means can be chosen by the library itself. That way we also don't leak the exact value into the API. WDYT?

That'd work I guess

[1proprogrammerchant]

I'd hope that libraries could use eventLoopGroup: any EventLoopGroup? = nil where nil means "default" and what that means can be chosen by the library itself. That way we also don't leak the exact value into the API. WDYT?

That'd work I guess

give it a try