HTTPClientRequest looks & feels like a value type but isn't

[weissi]

Sadly HTTPClientRequest contains a streamable body which is typically a reference to a (frequently) consume-once AsyncSequence. That means it's not actually a value type.

That's a major API winkle that should be fixed as soon as feasible.

My recommendation would be to separate the request and the body. Something like try await httpClient.execute(request, body: myStream).

[dnadoba]

Agree, same is true for the response. We should do it at the same time we adopt swift-http-types.

[weissi]

Indeed, swift-http-types adoption is the right cause of action here. Maybe we can even do this without a major then. Can we just deprecate HTTPClientRequest/Reponse and migrate to http types?

[dnadoba]

Currently yes but not after your PR has landed.

[weissi]

Currently yes but not after your PR has landed.

Oh really, what changes?

[dnadoba]

Currently HTTPClientRequest can be split into <Body: AsyncSequence>(HTTPRequest, Body) but afterwards it would be <Body: AsyncSequence>(HTTPRequest, Body, TLSConfiguration?).

[weissi]

Currently HTTPClientRequest can be split into <Body: AsyncSequence>(HTTPRequest, Body) but afterwards it would be <Body: AsyncSequence>(HTTPRequest, Body, TLSConfiguration?).

Cool, that seems to work either way.

[dnadoba]

It doesn't scale well. Why is TLSConfiguration now an argument but the rest of the HTTPClient.Configuration not? I think your proposed solution in https://github.com/swift-server/async-http-client/issues/392 is the way forward with the exception that tier 3 is just (HTTPRequest, Body) which is given to tier 2 to execute. If you want to change the TLSConfiguration you just mutate a local copy of your tier 2 value.

[weissi]

It doesn't scale well. Why is TLSConfiguration now an argument but the rest of the HTTPClient.Configuration not? I think the your proposed solution in #392 is the way forward with the exception that tier 3 is just (HTTPRequest, Body) which is given to tier 2 to execute. If you want to change the TLSConfiguration you just mutate a local copy of your tier 2 value.

Right, there's still an API-design challenge but #709 doesn't change anything about it because it amends the to-be-deprecated HTTPClientRequest.

[fumoboy007]

Agree, same is true for the response. We should do it at the same time we adopt swift-http-types.

Hi @dnadoba! Is this the issue that I should subscribe to for swift-http-types adoption or is there a separate issue?

[dnadoba]

That’s the right issue to track :)