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

Remove SPI from NIOAsyncChannel and new bootstrap methods #2548

Merged
merged 1 commit into from
Oct 17, 2023

Conversation

FranzBusch
Copy link
Member

Motivation

Over the past months, we have been working on new async bridges to make using NIO's Channel from Swift Concurrency possible. Since this work was far reaching we have opted to land all of it as SPI. Now the time has come and we feel confident enough to make the SPI official API. This comes after testing the new APIs in various scenarios such as HTTP 1&2, HTTP upgrades, protocol negotiation and in benchmarks.

Modification

This PR removes the SPI from the NIOAsyncChannel and the bootstrap methods. The other SPI usages will be removed in follow up PRs to keep the scope of the PRs relatively small.

Result

Everyone can use the NIOAsyncChannel 🚀

@Lukasa Lukasa added the semver/minor Adds new public API. label Oct 11, 2023
@FranzBusch FranzBusch force-pushed the fb-async-channel-api branch 3 times, most recently from 6d411a0 to a27b836 Compare October 16, 2023 14:40
try await channel.close()
try await channel.closeFuture.get()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is interesting. It took me a second to figure out what was going on but it looks like the compiler resolves to a different method when SPI was in play and it incorrectly resolved to the default init instead of the one overloaded with Outbound == Never. Removing the SPI changed that and the test correctly failed with Channel already closed

…gotiation and HTTP upgrade.

# Motivation
Over the past months, we have been working on new async bridges to make using NIO's `Channel` from Swift Concurrency possible. Since this work was far reaching we have opted to land all of it as SPI. Now the time has come and we feel confident enough to make the SPI official API. This comes after testing the new APIs in various scenarios such as HTTP 1&2, HTTP upgrades, protocol negotiation and in benchmarks.

# Modification
This PR removes the SPI from the `NIOAsyncChannel`, the bootstrap methods, protocol negotiation and HTTP upgrade.

# Result
Everyone can use the our new APIs🚀
@FranzBusch
Copy link
Member Author

@Lukasa @glbrntt This PR now includes the removal of all SPI in this repo. I decided against splitting it because diffs are really small always (just removing the SPI bits) and you can't review the actual API changes in this PR anyhow. That's what my previous PR #2549 was for.

Copy link
Contributor

@glbrntt glbrntt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@glbrntt glbrntt merged commit f38b7fd into apple:main Oct 17, 2023
7 of 8 checks passed
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.

3 participants