Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deprecate NIOEventLoopGroupProvider.createNew #2480

Merged
merged 1 commit into from
Nov 25, 2024
Merged

Conversation

weissi
Copy link
Member

@weissi weissi commented Jul 21, 2023

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.createNew
  • soft-deprecate (document as deprecated but don't mark @available(deprecated)) NIOEventLoopGroupProvider

Result:

@weissi weissi marked this pull request as draft July 21, 2023 15:10
@weissi
Copy link
Member Author

weissi commented Jul 21, 2023

Must be merged after #2471

@weissi weissi added the semver/minor Adds new public API. label Jul 21, 2023
@weissi weissi marked this pull request as ready for review July 26, 2023 23:05
@adam-fowler
Copy link
Contributor

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.

@weissi weissi marked this pull request as draft July 27, 2023 09:45
@weissi
Copy link
Member Author

weissi commented Jul 27, 2023

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
Copy link
Contributor

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
Copy link

1proprogrammerchant commented Aug 2, 2023

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

@weissi weissi marked this pull request as ready for review November 23, 2024 16:23
@weissi
Copy link
Member Author

weissi commented Nov 23, 2024

friendly ping @Lukasa

@weissi weissi force-pushed the jw-2142 branch 2 times, most recently from 6494dee to 9254ec0 Compare November 25, 2024 17:48
@weissi weissi requested a review from glbrntt November 25, 2024 17:48
@weissi weissi changed the title deprecate NIOEventLoopGroupProvider (#2142) deprecate NIOEventLoopGroupProvider.createNew (#2142) Nov 25, 2024
@weissi weissi changed the title deprecate NIOEventLoopGroupProvider.createNew (#2142) deprecate NIOEventLoopGroupProvider.createNew Nov 25, 2024
@weissi
Copy link
Member Author

weissi commented Nov 25, 2024

@Lukasa / @glbrntt addressed, ptal

Sources/NIOCore/EventLoop.swift Outdated Show resolved Hide resolved
Sources/NIOCore/EventLoop.swift Outdated Show resolved Hide resolved
Sources/NIOCore/EventLoop.swift Outdated Show resolved Hide resolved
@weissi weissi force-pushed the jw-2142 branch 2 times, most recently from a54a324 to 113d3fb Compare November 25, 2024 17:57
@weissi weissi enabled auto-merge (squash) November 25, 2024 18:09
@weissi weissi merged commit 848e428 into apple:main Nov 25, 2024
42 of 43 checks passed
@weissi weissi deleted the jw-2142 branch November 26, 2024 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver/minor Adds new public API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ecosystem issue: EventLoopGroupProvider and the 'similar create or share ELG' pattern
6 participants