Conversation
e134261 to
dc82f84
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds optional AsyncHTTPClient support to EventSource through Swift package traits (Swift 6.1+). The implementation provides an automatic fallback mechanism on Linux where EventSource can switch from URLSession to AsyncHTTPClient if URLSession encounters retryable errors, while maintaining backward compatibility through the Package@swift-6.0.swift file.
Changes:
- Adds AsyncHTTPClient as an optional dependency via package traits
- Implements fallback mechanism from URLSession to AsyncHTTPClient on Linux for retryable errors
- Adds test coverage for fallback policy logic
- Updates CI to test both default and AsyncHTTPClient configurations
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| Package.swift | Updates swift-tools-version to 6.1, adds AsyncHTTPClient trait and conditional dependency |
| Package@swift-6.0.swift | Provides backward compatibility for Swift 6.0 without trait support |
| Sources/EventSource/EventSource+AsyncHTTPClient.swift | Implements AsyncHTTPClient backend, streaming protocol, and fallback policy |
| Sources/EventSource/EventSource.swift | Refactors connection logic to support fallback mechanism and extracts reusable methods |
| Tests/EventSourceTests/AsyncHTTPClientTraitTests.swift | Adds tests for fallback policy and protocol conformance |
| README.md | Documents AsyncHTTPClient trait usage and limitations |
| .github/workflows/ci.yml | Adds AsyncHTTPClient variant testing to CI pipeline |
| Package.resolved | Adds AsyncHTTPClient and its transitive dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
Package.swift:45
- The testTarget "EventSourceTests" should also conditionally depend on AsyncHTTPClient when the trait is enabled. The tests in AsyncHTTPClientTraitTests.swift import and use AsyncHTTPClient, NIOCore, NIOHTTP1, and NIOPosix, but these dependencies are not declared in the test target.
Add the AsyncHTTPClient dependency to the testTarget with the same trait condition to ensure the tests can compile when the trait is enabled.
.testTarget(
name: "EventSourceTests",
dependencies: ["EventSource"]
),
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Related to huggingface/AnyLanguageModel#127 (comment)
This PR takes a similar approach as mattt/Replay#10