Support NIO Transport Services - part 2 #184
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Can one of the admins verify this patch? |
2 similar comments
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Here's a list of all the failing tests. Some of them fail both with TS and without. With Transport Services
Without
|
|
fyi Set Environment variable "ENABLE_TS_TESTS" to "true" to test NIOTS. |
artemredkin
reviewed
Mar 30, 2020
artemredkin
reviewed
Mar 30, 2020
artemredkin
reviewed
Mar 30, 2020
artemredkin
reviewed
Mar 30, 2020
artemredkin
reviewed
Mar 30, 2020
artemredkin
reviewed
Mar 30, 2020
artemredkin
reviewed
Mar 30, 2020
artemredkin
reviewed
Mar 30, 2020
artemredkin
reviewed
Mar 30, 2020
| let group = MultiThreadedEventLoopGroup(numberOfThreads: 4) | ||
| let group = getDefaultEventLoopGroup(numberOfThreads: 4) | ||
| defer { | ||
| XCTAssertNoThrow(try group.syncShutdownGracefully()) |
There was a problem hiding this comment.
shouldn't this be executed by the tests teardown?
There was a problem hiding this comment.
In this case the eventLoopGroup has been created inside the test because it required a different number of threads
artemredkin
reviewed
Mar 30, 2020
artemredkin
reviewed
Mar 30, 2020
artemredkin
reviewed
Mar 30, 2020
adam-fowler
force-pushed
the
niots
branch
3 times, most recently
from
14a3a63 to
e57d8bb
Compare
adam-fowler
force-pushed
the
niots
branch
2 times, most recently
from
55e3c18 to
c090008
Compare
Added NWTLSError, NWDNSError and NWPOSIXError to wrap Network.framework errors. The reason to do this was to make it easier to read Errors thrown from the Network.framework. Added NWError tests
Added macOS 10.14 fallbacks for min/max TLS protocol Enabled applicationProtocols code, now uses `String.withCString`
Removed setting option `waitForActivity` to false If running with a proxy don't setup NIOTSClientTLSProvider While running NIOTS ignore SSL unclean shutdown tests
|
@swift-server-bot test this please |
artemredkin
approved these changes
Apr 17, 2020
weissi
requested changes
Apr 17, 2020
Sources/AsyncHTTPClient/NIOTransportServices/NWErrorHandler.swift
Outdated
Show resolved
Hide resolved
Sources/AsyncHTTPClient/NIOTransportServices/NWErrorHandler.swift
Outdated
Show resolved
Hide resolved
Also don't need to create a tlsConfiguration if there is a proxy so moved its initialisation down a bit
weissi
reviewed
Apr 17, 2020
|
|
||
| /// Are we testing NIO Transport services | ||
| func isTestingNIOTS() -> Bool { | ||
| return ProcessInfo.processInfo.environment["ENABLE_TS_TESTS"] == "true" |
There was a problem hiding this comment.
@adam-fowler one last question: Could we flip the default here and have the tests run on TS by default? Otherwise I'm sure nobody will ever run them :|
weissi
approved these changes
Apr 17, 2020
|
@swift-server-bot add to whitelist |
|
hit #175 |
|
retrying |
|
@swift-server-bot test this please |
|
@adam-fowler I think the already opened PR #192 should fix #175 while also making the code simpler |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the continuation of the good work of @Yasumoto and @weissi in #135
The following code adds support for NIO Transport services. When the
ConnectionPoolasks for a connection bootstrap it is returned aNIOClientTCPBootstrapwhich wraps either aNIOTSConnectionBootstrapor aClientBootstrapdepending on whether the EventLoop we are running on isNIOTSEventLoop.If you initialize an
HTTPClientwitheventLoopGroupProviderset to.createNewthen if you are running on iOS, macOS 10.14 or later it will provide aNIOTSEventLoopGroupinstead of aEventLoopGroup.Currently a number of tests are failing. 4 of these are related to the NIOSSLUncleanShutdown error the others all seem related to various race conditions which are being dealt with on other PRs. I have tested this code with aws-sdk-swift and it is working on both macOS and iOS.
Things look into: