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

Add NIOT #95

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Add NIOT #95

wants to merge 5 commits into from

Conversation

hishnash
Copy link

Add NIOTransportServices support on Apple platforms.

On apple platforms it is best to use the NIOTSEventLoopGroup and use the corresponding SSL configurations.

This PR also adds a HTTPChannelIntercepter that can be used to inspect the HTTP handshake requests (for locking etc).

@@ -1,4 +1,4 @@
// swift-tools-version:5.2
// swift-tools-version:5.3
Copy link
Member

Choose a reason for hiding this comment

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

We can't change the tools version as we need to keep support for 5.2 - does that block the PR?

Copy link
Author

Choose a reason for hiding this comment

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

I believe 5.3 is require for the

condition: .when(
                     platforms: [Platform.iOS, Platform.macOS, Platform.tvOS, Platform.watchOS]
                 )

If this is a blocker then i might look into how i could make changes to this package so that an additional package could provide the NIOTransportServices decency. (either through a subclass or a plugin like solution). what do you think would be the best approach?

Copy link
Member

Choose a reason for hiding this comment

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

Add a [email protected] manifest which contains the old version and then that should work (we might need to wrap the new methods in a Swift version check as well)

Copy link
Author

Choose a reason for hiding this comment

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

ok i will give that a go, should i also update the GitHub workflow so that it includes both versions of swift?

Copy link
Member

Choose a reason for hiding this comment

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

@gwynne could you update the CI with all the new fun stuff?

@hishnash
Copy link
Author

@0xTim I added the checks and package file for 5.2 i think the version checks i did are also in the correct places. (i did think that this might fail if for some reason someone has a package called Network installed on linux but maybe not im not sure how the compiler works with these canImport stuff should i also but a platform condition in place?

@0xTim
Copy link
Member

0xTim commented Jun 30, 2021

@hishnash I think canImport should be fine

@hishnash
Copy link
Author

@0xTim cool are there any other changes i need to make.

@0xTim
Copy link
Member

0xTim commented Jul 25, 2022

@hishnash sorry I've just spotted this! Sorry about the massive e delay. Is this now solved by #107 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants