Skip to content

Comments

Streaming federator#1966

Merged
pcapriotti merged 16 commits intodevelopfrom
pcapriotti/federator-cargohold
Dec 14, 2021
Merged

Streaming federator#1966
pcapriotti merged 16 commits intodevelopfrom
pcapriotti/federator-cargohold

Conversation

@pcapriotti
Copy link
Contributor

@pcapriotti pcapriotti commented Dec 6, 2021

This PR is about allowing federator and federator client to stream responses to their clients. Instead of using explicit CPS everywhere, this is achieved by replacing IO with Codensity IO in the interpreters for the Service and Remote effects. The streaming response type is StreamingResponse from Servant, even when using the low-level HTTP2 client API (withHTTP2Request).

The underlying SourceT type is slightly more complicated than necessary for our purposes (e.g. we do not need its Error and Skip constructors), but it is convenient not to have to define our own streaming abstraction.

Tracked by https://wearezeta.atlassian.net/browse/FS-322.

Checklist

  • The PR Title explains the impact of the change.
  • The PR description provides context as to why the change should occur and what the code contributes to that effect. This could also be a link to a JIRA ticket or a Github issue, if there is one.
  • changelog.d contains the following bits of information (details):
    • A file with the changelog entry in one or more suitable sub-sections. The sub-sections are marked by directories inside changelog.d.

@pcapriotti pcapriotti changed the title Federated cargohold Federated cargohold [skip ci] Dec 6, 2021
@pcapriotti pcapriotti force-pushed the pcapriotti/federator-cargohold branch from 9c03784 to 1fb1b82 Compare December 6, 2021 09:42
@pcapriotti pcapriotti changed the title Federated cargohold [skip ci] Federated cargohold Dec 6, 2021
@supersven supersven force-pushed the pcapriotti/federator-cargohold branch from 1e40303 to 05d3567 Compare December 6, 2021 18:10
@pcapriotti pcapriotti force-pushed the pcapriotti/federator-cargohold branch 2 times, most recently from 8064578 to a4e3b29 Compare December 7, 2021 10:27
@pcapriotti pcapriotti force-pushed the pcapriotti/federator-cargohold branch from a4e3b29 to 915c73e Compare December 7, 2021 16:15
pcapriotti and others added 6 commits December 8, 2021 07:59
Also remove use of polysemy-mock from ExternalServer tests.
Co-authored-by: Sven Tennie <sven.tennie@gmail.com>
This implements the bracketing pattern needed for closing a streaming
response at the level of the base monad, instead of introducing CPS for
the main polysemy actions.
@pcapriotti pcapriotti changed the title Federated cargohold Streaming federator Dec 8, 2021
@pcapriotti pcapriotti force-pushed the pcapriotti/federator-cargohold branch from 915c73e to 8a036b6 Compare December 8, 2021 07:02
@pcapriotti pcapriotti force-pushed the pcapriotti/federator-cargohold branch from a81c958 to 7f193e9 Compare December 8, 2021 10:31
@pcapriotti pcapriotti marked this pull request as ready for review December 13, 2021 07:56
deriving (Eq, Show)

mockService ::
Members [Output Call, Embed IO] r =>
Copy link
Contributor

@mdimjasevic mdimjasevic Dec 14, 2021

Choose a reason for hiding this comment

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

How come this works by using a value-level list? I was expecting a type-level list here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not a value level list. The promotion tick is optional when the type is unambiguous. For the case of lists, this means that [A, B] is interpreted as something of kind [*] (i.e. the same as '[A, B], aka A ': B ': '[]), but [A] is interpreted as something of type * (i.e. the type of lists of A).

We can turn on -Wunticked-promoted-constructors to turn this into an error, if desired. Should we?

@pcapriotti pcapriotti merged commit 9d1ae79 into develop Dec 14, 2021
@pcapriotti pcapriotti deleted the pcapriotti/federator-cargohold branch December 14, 2021 14:08
@akshaymankar akshaymankar mentioned this pull request Jan 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants