enable swift 6 2 jobs #2259
Closed
enable swift 6 2 jobs #2259
Conversation
This file contains hidden or 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
Motivation: v2 will be split across a number of packages, and v1 will be maintained on the 'release/1.x' branch. In order to do the package split this package needs to be whittled down to only the v2 components which will remain within this package. Modifications: - Remove v1 code - Remove v2 code destined for other packages - Reshuffle a few scripts Result: - Package contains only the intended code
Motivation: Standalone packages for examples are an easier on-ramp for newcomers as they only see the details they care about and it can act as a starting point which they can develop from. Modifications: - Turn each example into a standalone package and add a README to each - Update the generate and fetch scripts and re-run them Result: Better examples --------- Co-authored-by: Gus Cairo <[email protected]>
…2072) Motivation: A 'bar' package defining a 'FooService' service will have a number of types generateed for it, for example, `Bar_FooServiceServiceProtocol`, or `Bar_FooServiceClient`. The protocol buffers style guide reccomends that services are suffixed with "Service" (e.g. "FooService", not "Foo") which results in some strange protocol names (like `Bar_FooServiceServiceProtocol`). There should be a clearer differentiation between the qualified service name ("bar.FooServce") and the type being generated. The code generator already replaces "." with "_" in fully qualified names, so "_" is a natural choice as a separator. Modifications: - Add an "_" between the generated fully qualified service name and the gRPC component being generated - Update tests Result: Better names in generated code. The `Bar_FooServiceServiceProtocol` example above becomes `Bar_FooService_ServiceProtocol`, while `Bar_FooServiceClient` becomes `Bar_FooService_Client`.
Motivation: In grpc#2072 the code gen was changed to add an underscore to the generated names. This wasn't picked up in CI because the examples depend on "main" at the moment. Modifications: - Update examples Result: CI passes
Motivation: To create a connected pair of `ServerTransport` and `ClientTransport`, users need to call `InProcessTransport.makePair(serviceConfig:)` which returns a tuple. The spelling of this API can be more intuitive and simple. Modifications: - Restructure `InProcessTransport` to include two properties: `server` (the `ServerTransport`) and `client` (the `ClientTransport`), and an `init(serviceConfig:)` that pairs these properties. - Add missing GRPCInProcessTransport dependency to GRPCCore test target. Result: The spelling of the `InProcessTransport` API will be better.
Co-authored-by: Gus Cairo <[email protected]>
Motivation: The names of the request/response types are quite stilted. We can improve them by removing the namespacing and by removing the explicit "Single". Modifications: - `ServerRequest.Single` -> `ServerRequest` - `ServerRequest.Stream` -> `StreamingServerRequest` - `ServerResponse.Single` -> `ServerResponse` - `ServerResponse.Stream` -> `StreamingServerResponse` - `ClientRequest.Single` -> `ClientRequest` - `ClientRequest.Stream` -> `StreamingClientRequest` - `ClientResponse.Single` -> `ClientResponse` - `ClientResponse.Stream` -> `StreamingClientResponse` Result: Better naming
Motivation: Some names changed in grpc#2076 and now our examples don't compile. Modifications: - Update examples Result: Examples compile
Motivation: The examples depend on the main branch of our various packages. These can now use the tagged releases. Modifications: - Update dependencies in examples Result: Examples are up-to-date
Motivation: Core components of grpc-swift v2 require API from the latest SDKs. This causes a proliferation of availability annotations through our API. Rather than doing this we can set the minimum platforms in the package manifest. Modifications: - Remove availability annotations - Set platforms in the package manifest Result: - Less boilerplate - Users must set platforms in their package manifest
Motivation: For library maintainers and some users, having an understanding of how the library is structured is incredibly helpful. Modifications: - Add a high-level design doc linking to the relevant symbols - Fix up a couple of other DocC warnings Result: More docs --------- Co-authored-by: Gus Cairo <[email protected]>
## Motivation We want to be able to flatten `RPCError`s, and to do so we need to be able to merge the `Metadata` contained in each. ## Modifications This PR adds a helper function to merge one `Metadata` instance into another. ## Result Unblocks grpc#2083 and also provides a potentially useful API for users. **- Note:** Because of the way `Metadata` has been implemented, we can have multiple _identical_ key-value pairs. This isn't ideal, as it's particularly feasible that we'll end up with multiple repeated identical pairs when merging two `Metadata`s. I think we should reconsider the backing data structure (using a set for example) or add a check before inserting to avoid this.
The formatting script should print a helpful message when formatting check fails showing which command to run to fix it. However, it fails to print this because it cannot find the `THIS_SCRIPT` env variable.
…ode (grpc#2083) ## Motivation For errors happening deep in the task tree, we'd wrap them in many layers of `RPCError`s. This isn't particularly nice. ## Modifications This PR changes the behaviour of the `RPCError` initialiser to flatten the cause as long as it's an `RPCError` with the same status code as the wrapping error. ## Result Friendlier errors.
Motivation: As a service author it's useful to know if the RPC has been cancelled (because it's timed out, the remote peer closed it, the connection dropped etc). For cases where the stream has already closed this can be surfaced by a read or write failing. However, for cases like server-streaming RPCs where there are no reads and writes can be infrequent it's useful to have a more explicit signal. Modifications: - Add a `ServerCancellationManager`, this is internal per-stream storage for registering cancellation handlers and storing whether the RPC has been cancelled. - Add the `RPCCancellationHandle` nested within the `ServerContext`. This holds an instance of the manager and provides higher level APIs allowing users to check if the RPC has been cancellation and to wait until the RPC has been cancelled. - Add a top-level `withRPCCancellationHandler` which registers a callback with the manager. - Add a top-level `withServerContextRPCCancellationHandle` for creating and binding the task local manager. This is intended for use by transport implementations rather than users. - Update the in-process transport to cancel RPCs when shutting down gracefully. - Update the server executor to cancel RPCs when the timeout fires. Result: Users can watch for cancellation using `withRPCCancellationHandler`.
Motivation: MethodConfig uses a particular string based format for durations based on the "google.protobuf.duration" message. On some decoding paths a string was read and then decoded into a `Swift.Duration` rather than decoding the `GoogleProtobufDuration` message directly. The string-to-Swift.Duration path had a bug meaning fractional seconds were incorrectly decoded. Modifications: - Add a test - Remove the string to `Swift.Duration` path and always decode via `GoogleProtobufDuration` which has a correct implementation. Result: Fewer bugs
We changed some APIs and the tutorial code snippets have to be updated.
Co-authored-by: George Barnett <[email protected]>
) ### Motivation: * Reduce duplication * Centralise boilerplate changes when new Swift versions are picked up. * Benefit from centralised work to add new linting / test infrastructure. ### Modifications: Changes of note: * Use soundness checks from swiftlang/github-workflows. * Define a gRPC-specific soundness check which retains bespoke license-checking code for .swift files as the gRPC header style is very different to most templates and checks that generated code is up-to-date. ### Result: More test, linting, formatting coverage. More common CI with other Swift on Server projects.
Motivation: ManagedBuffer is explicitly not 'Sendable' on Swift nightly builds. As a result, our Lock type stops being 'Sendable'. This is only used by the 'BroadcastAsyncSequence'. Modifications: - Modify 'BroadcastAsyncSequence' to use 'Mutex'. One downside to this approach is that when a subscribe wants to consume an element and there's no element available, the lock must be released and then re-acquired once a continuation has been created. - Remove our lock, and update NOTICES.txt to reflect the fact we no longer use a copy of NIOs lock. Result: - Less code - CI passes
Motivation: We added '@_exported import GRPCCore' to transport modules to make it easier for users as it saves them from adding an import. On reflection we've our position on this has changed and the implicit dependency will likely make things less clear. Modifications: - Remove exported import from in-proc transport - Update tutorials/examples to have explicit 'GRPCCore' imports Result: More explicit dependencies
Motivation: The test targets don't set the same Swift as the library targets. There's no reason for them not to. Modifications: Apply default settings to test targets. Result: Test targets use Swift 6 language mode, explicit existential any, and internal imports by default
There were some warnings when building the documentation so this PR fixes them.
…rpc#2096) ## Motivation We want to allow users to customise the RPCs a registered interceptor should apply to on the server: - Intercept all requests - Intercept requests only meant for specific services - Intercept requests only meant for specific methods ## Modifications This PR adds a new `ServerInterceptorTarget` type that allows users to specify what the target of the interceptor should be. Existing APIs accepting `[any ServerInterceptor]` have been changed to instead take `[ServerInterceptorTarget]`. ## Result Users can have more control over to which requests interceptors are applied. --------- Co-authored-by: George Barnett <[email protected]>
Motivation: The structured Swift types aren't marked as Sendable. This means that defining a 'static let' isn't possible, and a static computed property must be used instead. This is fine, just verbose. Modifications: - Make all structured swift representation types 'Sendable' Result: Can use 'static let' with structured Swift types
Motivation: If a blank line is render, the text based renderer will include the current indentation level resulting in lines with just spaces. Modifications: - Don't include indentation if the line to render is otherwise empty Result: Less trailing whitespace.
Motivation: A number of types are nested within 'CodeGenerationRequest' which make them verbose to type out and a bit of a pain to work with. Modification: Remove the nesting of types. Result: Easier types to work with.
…rpc#2113) ## Motivation We want to allow users to customise the RPCs a registered interceptor should apply to on the client: - Intercept all requests - Intercept requests only meant for specific services - Intercept requests only meant for specific methods ## Modifications This PR adds a new `ClientInterceptorPipelineOperation` type that allows users to specify what the target of the interceptor should be. Existing APIs accepting `[any ClientInterceptor]` have been kept, but new initialisers taking `[ClientInterceptorPipelineOperation]` instead have been added. ## Result Users can have more control over to which requests interceptors are applied.
Motivation: The server response types have a metadata computed property with only a getter. It's entirely possible to mutate the metadata manually, it's just a bit of a faff. This should be easier. Modifications: - Add a setter to the computed property - Migrate server response tests to swift-testing Result: - Easier to use API
) Motivation: The code gen as it stands tests too much in one go: a small change leads to many tests being updated. This stems from the translator not being very testable. To improve this, and make future code gen changes less painful, we can refactor it such that helpers build structured swift. These are significantly less coupled and can be tested more easily. The strategy here is to add all the structured representations for the metadata, client, and server and then cut over the translators to using the new code. This first change will introduce the structured represntation for metadata. Modifications: - Add metadata structured swift - Add helpers for commonly used types Result: Metadata is in place
This PR changes `Metadata`'s description to include quotes around string
values.
From this:
```
[("some-key", some-value)]
```
to this:
```
[("some-key", "some-value")]
```
---------
Co-authored-by: George Barnett <[email protected]>
Motivation: The CI for examples builds them in the setup stage and the actual run stages. That's just a silly oversight. Modifications: - Don't build them during set. Result: Examples don't build twice in CI
…2190) Motivation: The compatibility doc doesn't mention minimum deployment versions for Apple platforms. It should. Modifications: - Add minimum deployment versions. Result: Better docs.
Motivation: The imports should all have access level set explicitly. Some were missing. Modifications: - Add a script and CI to check for this - Add a few missing access levels Result: Better consistency
Modifications: - Add config to the code generator allowing a different module name to be specified. - Remove unused code Result: The grpc module name can be specified in generated code
Motivation: Moving from 1.x to 2.x isn't trivial. We should provide a guide for doing this. Modifications: - Add a guide for migrating client code and services - Add a script which can automate parts of this Result: Some guidance on migrating. --------- Co-authored-by: Gus Cairo <[email protected]>
Rename nightly_6_1 params to nightly_next; see apple/swift-nio#3122
This PR fixes some imports so that grpc-swift can be built using the static Linux SDK.
Add static SDK CI workflow which runs on commits to PRs, merges to main and daily on main. --------- Co-authored-by: Gus Cairo <[email protected]> Co-authored-by: George Barnett <[email protected]>
This PR follows-up grpc#2206 and fixes the Android build I noticed at https://swift-everywhere.org. Hopefully we will soon have CI support for Android (swiftlang/github-workflows#106) to catch this sort of breakage sooner.
Motivation: gRPC checks whether errors thrown from interceptors are `RPCError` and otherwise treats them as `unknown` (to avoid leaking internal information). There is a third possibility: the error is explicitly marked as being convertible to an `RPCError`. This check is currently missing when thrown from client/server interceptors. Modifications: - Catch `RPCErrorConvertible` in the client/server executors when thrown from interceptors - Add tests Result: Error information isn't dropped
Motivation: GRPCClient can throw an error when run is called more than once or if run is called after it has been shutdown. Normally this would happen if a user caller 'run()' more than once, but can also happen if 'withGRPCClient' is called and the client is never used and the body returns quickly. Modifications: - Improve the error message Result: Better error message.
Motivation: Swift 6.1 has been released, we should add it to our CI coverage. Modifications: - Add additional Swift 6.1 jobs where appropriate in main.yml, pull_request.yml - Add thresholds for Swift 6.1 Result: Improved test coverage.
`Task.sleep` will by default try and coalesce multiple timers into one, mostly for client-specific reasons such as performance, power consumption, etc. However, this is undesirable on servers, as it can increase latency, memory usage, and (in the case of gRPC) may result in timeouts not firing when they should. We can avoid this by setting the sleep `tolerance` to zero.
Motivation: We should aim to keep warnings at zero. There were a few littered around as a result of Swift 6.1. Modifications: - Fix the warnings - Enabled warnings-as-errors in CI to avoid regressions Result: No warnings
Motivation: Currently there's no way to plumb through details from the transport level to a request handler. Adding this field allows transports, such as the nio transport, to add the peer certificate to the server context when using mTLS. From there there it's easy for an interceptor to take this data and propogate it forward to a request handler. Modifications: This PR adds a single field to the ServerContext that transports can use Result: A new field will be accessible to transports and consumers of the API --------- Co-authored-by: George Barnett <[email protected]>
Motivation: gRPC is moving from specifying the platforms in the package manifest to annotating code with availability annotations. In order to do this the generateed code must also be annotated. Modifications: - Generate appropriate annotations Result: Generated code has availability annotations --------- Co-authored-by: Gus Cairo <[email protected]>
Motivation: gRPC is moving from specifying the platforms in the package manifest to annotating code with availability annotations. Modifications: - Explicitly annotate public API - Add '-require-explicit-availability' to CI - Drop platforms Result: - gRPC can be used by packages which don't specify their platforms in the manifest
Motivation: `-require-explicit-sendable` is ineffective without `-Xfrontend` but combining it with `-warnings-as-errors` surfaces a warning from SwiftPM. Modifications: - remove `-require-explicit-sendable`, it's not doing anything - add `-require-explicit-availability` to nightly jobs Result: More consistent CI flags
Motivation: Binary metadata values are encoded as base64 strings. The gRPC spec doesn't require that the values are padded. Currently gRPC Swift requires values to be padded otherwise decoding will fail. Modifications: - Allow padding characters to be omitted when decoding base64 Result: Can decode unpadded binary metadata values
Motivation: v2 has moved to a new repo, grpc-swift-2. That's not all that discoverable. Modifications: - Deprecate commonly used high-level types with a link to a forums post explaining the move. Result: Users are notified about the move
Motivation: Swift 6.2 has been released, we should add it to our CI coverage. Modifications: Add additional Swift 6.2 jobs where appropriate in main.yml, pull_request.yml Result: Improved test coverage.
|
Sorry for the noise |
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.
A CI update script fell afoul of the v2 repo changes