Add swift-configuration support by hamzahrmalik · Pull Request #878 · swift-server/async-http-client

hamzahrmalik · 2026-01-08T15:57:59Z

Users should be able to use swift-configuration to create the http client configuration object

Changes

duplicate package.swift to create a separate version for 6.0 and 6.1, since Configuration is 6.2+
add helper to create http client configuration using ConfigReader

Sources/AsyncHTTPClient/HTTPClientConfiguration+SwiftConfiguration.swift

czechboy0 · 2026-01-08T16:01:02Z

Sources/AsyncHTTPClient/HTTPClientConfiguration+SwiftConfiguration.swift

+    public init(configReader: ConfigReader) throws {
+        self.init()
+
+        // Each entry in the list should be a colon separated pair e.g. localhost:127.0.0.1 or localhost:::1


Why not comma-separated?

colon felt more natural to me for defining key-value pairs. I don't mind though

comma would probably make the ipv6 case slightly eaiser to parse too

Oh right, comma might be used for splitting the pairs themselves e.g. in env vars. Hmm then comma isn't great, but colon is a bit confusing.

czechboy0 · 2026-01-12T13:30:26Z

lgtm, I'll let the maintainers actually approve though 🙂

FranzBusch · 2026-01-13T10:53:51Z

Sources/AsyncHTTPClient/HTTPClientConfiguration+SwiftConfiguration.swift

+    /// - `dnsOverrides` (string array, optional): Colon-separated host:IP pairs for DNS overrides (e.g., "localhost:127.0.0.1").
+    /// - `redirect` (scoped): Redirect handling configuration read by ``RedirectConfiguration/init(configReader:)``.
+    /// - `timeout` (scoped): Timeout configuration read by ``Timeout/init(configReader:)``.
+    /// - `connectionPool` (scoped): Connection pool configuration read by ``ConnectionPool/init(configReader:)``.
+    /// - `httpVersion` (string, optional, default: automatic): HTTP version to use ( "automatic" or  "http1Only").
+    /// - `maximumUsesPerConnection` (int, optional, default: nil, no limit): Maximum uses per connection.


Should we do camelcase or dot separated? In Temporal we choose the latter https://github.com/apple/swift-temporal-sdk/blob/eb356f25b53d4ac50b1505dea1b7306ba7296bc8/Sources/Temporal/Client/TemporalClient%2BConfiguration.swift#L111

I used dots for hierarchy but camel case for words

dns.overrides and http.version make sense in that system. But maximum.uses.per.connection does not

This could be connection.limits.maximum.uses. It is not the end of the world but it would be great if we could be a bit consistent around the ecosystem. @czechboy0 WDYT?

oh yeah i like that idea

I've been doing the same as Hamzah - use dots for separating components, with individual components being camelCase. So http.maxConnectionCount, for example.

Should we document this somewhere? It feels like that would be good to have a recommendation at least for the ecosystem.

apple/swift-configuration#145

Feel free to add more questions to the issue, will add docs on this

fabianfett · 2026-01-23T09:56:43Z

Tests/AsyncHTTPClientTests/SwiftConfigurationTests.swift

+        let testProvider = InMemoryProvider(values: [
+            "dnsOverrides": .init(.stringArray(["invalidentry", "localhost:127.0.0.1"]), isSecret: false)
+        ])


what about spaces:

localhost: 127.0.0.1

Are we fine with the silent error here?

i will make it trim spaces, i think that's going to be the least surprising behaviour

do we think it's preferable to throw an error on invalid config? It would make it much more clear i suppose

Yeah I think throwing an error if the value of a key isn't valid is very reasonable

fabianfett · 2026-01-23T09:58:50Z

Tests/AsyncHTTPClientTests/SwiftConfigurationTests.swift

+    @Test
+    @available(macOS 15.0, iOS 18.0, watchOS 11.0, tvOS 18.0, visionOS 2.0, *)
+    func singleDnsOverride() throws {
+        let testProvider = InMemoryProvider(values: [
+            "dnsOverrides": .init(.stringArray(["api.example.com:10.0.0.1"]), isSecret: false)
+        ])
+
+        let configReader = ConfigReader(provider: testProvider)
+        let config = try HTTPClient.Configuration(configReader: configReader)
+
+        #expect(config.dnsOverride["api.example.com"] == "10.0.0.1")
+        #expect(config.dnsOverride.count == 1)
+    }


what's the point of this test? what is tested here, that the other tests miss?

it's not needed. i think i wrote this one first but the other ones cover this and more so i'll remove it

Package@swift-6.0.swift

fabianfett · 2026-01-23T13:24:22Z

Am I correctly assuming, that this new behavior is only available to newly created clients and not the default shared one? If yes, is this the intended behavior?

hamzahrmalik · 2026-01-23T13:29:10Z

Am I correctly assuming, that this new behavior is only available to newly created clients and not the default shared one? If yes, is this the intended behavior?

The changes in this PR allow you to create a configuration from a configuration reader

You can then use that configuration to make a client

The shared client does not support a custom configuration. Therefore you are correct that the changes in this PR do not apply to the singleton

FranzBusch · 2026-01-23T13:44:56Z

The shared client does not support a custom configuration. Therefore you are correct that the changes in this PR do not apply to the singleton

I think this is expected for now. We could think about letting the singleton client use the EnviornmentConfigProvider in the future but I would keep that in a separate PR to think about it more.

fabianfett · 2026-01-30T12:14:45Z

Sources/AsyncHTTPClient/HTTPClientConfiguration+SwiftConfiguration.swift

+    /// - `allowCycles` (bool, optional, default: false): Allow cyclic redirects when mode is "follow".
+    ///
+    /// - Throws: `HTTPClientError.invalidRedirectConfiguration` if mode is specified but invalid.
+    public init(configReader: ConfigReader) throws {


I'm not sure about the developer experience, here I think it would be better to keep this method internal and doc all the keys in the OG init. WDYT?

@czechboy0 do you have any guidance for this?

We just have this: https://swiftpackageindex.com/apple/swift-configuration/main/documentation/configuration/configuring-libraries

But it doesn't go into what to do with nested structs.

My rule of thumb would be: if the nested structs are public API, and you can construct them using public API, I think the config-based initializer should then also be public. Otherwise we move those docs onto a parent struct that isn't easy to find unless you already know where to find it. So I think it's just easier to follow this way.

Add swift-configuration support

1a08982

czechboy0 reviewed Jan 8, 2026

View reviewed changes

hamzahrmalik added 2 commits January 9, 2026 15:30

Add docs

16f81b6

code review

4fd162a

FranzBusch reviewed Jan 13, 2026

View reviewed changes

FranzBusch approved these changes Jan 19, 2026

View reviewed changes

hamzahrmalik added 2 commits January 19, 2026 10:31

Add compiler guards

28c2723

license headers

8049d8d

czechboy0 added the 🆕 semver/minor Adds new public API. label Jan 21, 2026

FranzBusch approved these changes Jan 22, 2026

View reviewed changes

fabianfett reviewed Jan 23, 2026

View reviewed changes

Package@swift-6.0.swift Show resolved Hide resolved

allow whitespace in dns overrides

0905177

throw on invalid dns config

4c55cd2

fabianfett reviewed Jan 30, 2026

View reviewed changes

Merge branch 'main' into swift_config

ae0ad23

fabianfett approved these changes Feb 4, 2026

View reviewed changes

fabianfett merged commit 986dc47 into swift-server:main Feb 5, 2026
62 of 63 checks passed

Comments

Conversation

hamzahrmalik commented Jan 8, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

czechboy0 commented Jan 12, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fabianfett commented Jan 23, 2026

Uh oh!

hamzahrmalik commented Jan 23, 2026

Uh oh!

FranzBusch commented Jan 23, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

czechboy0 Jan 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

czechboy0 Jan 30, 2026 •

edited

Loading