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.

Sign up for GitHub

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

Jump to bottom

Feature/async await #71

Draft
wants to merge 20 commits into
base: main
Choose a base branch
from
Draft

Feature/async await #71

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
cbd8f48
feature: Start async await support
paulofaria Oct 28, 2021
1091954
feature: Update README.md
paulofaria Oct 29, 2021
24b8cab
fix: Fix sdk checks
paulofaria Oct 29, 2021
6b56632
merge: Fix merge conflict
paulofaria Oct 29, 2021
bb8a9ff
feature: Add async
paulofaria Oct 29, 2021
5741a56
refactor: Start replacing deprecated APIs in tests
paulofaria Oct 29, 2021
377a539
chore: Update CI to Swift 5.5
paulofaria Oct 29, 2021
6faaba7
chore: Fix GraphQl dependency in Package.swift
paulofaria Oct 29, 2021
d051b4b
refactor: Improve counter example
paulofaria Oct 30, 2021
b1f6ec3
fix: Fix README.md
paulofaria Oct 30, 2021
a561f17
fix: Fix README.md
paulofaria Oct 30, 2021
1e527a5
fix: Fix README.md
paulofaria Oct 30, 2021
cf57da4
feature: Add SDL to README.md
paulofaria Oct 30, 2021
90cdabe
refactor: Fix counter state naming
paulofaria Oct 30, 2021
3ae8d0a
feature: String literal descriptions
paulofaria Nov 1, 2021
69a2cbd
feature: Start directive feature
paulofaria Nov 1, 2021
1581a7f
feature: Server directive support
paulofaria Nov 1, 2021
1b1bd09
feature: Experiment with directive support
paulofaria Nov 2, 2021
2285f01
feature: Improve directives
paulofaria Nov 3, 2021
50f4ae9
feature: Improve directive support
paulofaria Nov 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ jobs:
- focal
- bionic
tag:
- swift:5.4
- swift:5.5
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason you are forcing everyone to upgrade to swift 5.5? Most other initial implementations of async/await in existing libraries have been fairly slim additions on top of what already existed. And those additions hidden behind a #if compiler(>=5.5) && canImport(_Concurrency) thus removing the swift 5.5 requirement for the whole package. Another common practice has been to move all the new concurrency code into its own folder so it can easily be found.

Checkout the implementations from Vapor, gRPC and my own Soto.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hey, @adam-fowler. I just did that because I was having a hard time making things work and that was one of my attempts, 🙃. I saw that that #if compiler(>=5.5) && canImport(_Concurrency) check is the way to go. I'm actually using it already. Moving all the concurrency code into a separate folder sounds like a good idea. Another thing that I'm a bit confused is that Xcode keeps yelling me about @available(macOS 12, *) and, for now, I'm adding those all over the place so I can keep focusing on the work itself. Those are mostly sprinkled all over the tests. Are those really necessary?

My plan is to not make a breaking change with this async-await support. However, I want all the tests to be using this new API. I'm not sure it's worth to duplicate all tests, one for each version of the API. That's why I updated the CI requirement to Swift 5.5. I could add a new build variant with Swift 5.4 to the CI configuration, just to be sure we're building. I'm not sure there will be any tests left which don't require Swift 5.5, though.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you move all your code to a separate folder you can add the @available check to the extension of each type instead of every function.

Yeah the question of testing is a little awkward. For SotoCore I ended up leaving most of my tests using the EventLoopFuture apis. And just added a single test for each async function. That worked fine for me but then SotoCore async code had very few additional functions. Again I did this in a separate file.

One thing to also note, the test discovery on Linux doesn't deal with async functions. This will be fixed but in the meantime I ended up added a helper function that kicked off a Task and used DispatchGroup to wait until that Task was complete.

@available(macOS 12.0, iOS 15.0, watchOS 8.0, tvOS 15.0, *)
public func XCTRunAsyncAndBlock(_ closure: @escaping () async throws -> Void) {
    let dg = DispatchGroup()
    dg.enter()
    Task {
        do {
            try await closure()
        } catch {
            XCTFail("\(error)")
        }
        dg.leave()
    }
    dg.wait()
}

container:
image: ${{ matrix.tag }}-${{ matrix.os }}
steps:
Expand Down
8 changes: 6 additions & 2 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
.DS_Store

### SwiftPM ###
/.build
/.swiftpm
.build/
.swiftpm/

### Jetbrains ###
.idea/

17 changes: 4 additions & 13 deletions Package.resolved
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,31 +1,22 @@
{
"object": {
"pins": [
{
"package": "GraphQL",
"repositoryURL": "https://github.com/GraphQLSwift/GraphQL.git",
"state": {
"branch": null,
"revision": "e5de315125f8220334ba3799bbd78c7c1ed529f7",
"version": "2.0.0"
}
},
{
"package": "swift-collections",
"repositoryURL": "https://github.com/apple/swift-collections",
"state": {
"branch": null,
"revision": "d45e63421d3dff834949ac69d3c37691e994bd69",
"version": "0.0.3"
"revision": "2d33a0ea89c961dcb2b3da2157963d9c0370347e",
"version": "1.0.1"
}
},
{
"package": "swift-nio",
"repositoryURL": "https://github.com/apple/swift-nio.git",
"state": {
"branch": null,
"revision": "d161bf658780b209c185994528e7e24376cf7283",
"version": "2.29.0"
"revision": "6aa9347d9bc5bbfe6a84983aec955c17ffea96ef",
"version": "2.33.0"
}
}
]
Expand Down
6 changes: 4 additions & 2 deletions Package.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// swift-tools-version:5.4
// swift-tools-version:5.3
import PackageDescription

let package = Package(
Expand All @@ -7,7 +7,9 @@ let package = Package(
.library(name: "Graphiti", targets: ["Graphiti"]),
],
dependencies: [
.package(url: "https://github.com/GraphQLSwift/GraphQL.git", .upToNextMajor(from: "2.0.0"))
// .package(url: "https://github.com/GraphQLSwift/GraphQL.git", .upToNextMajor(from: "2.0.0")),
// .package(url: "https://github.com/GraphQLSwift/GraphQL.git", .branch("feature/async-await")),
.package(path: "../GraphQL")
],
targets: [
.target(name: "Graphiti", dependencies: ["GraphQL"]),
Expand Down