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.

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

Refactor http calls to support message payloads #646

Merged
merged 3 commits into from
Dec 6, 2023

Conversation

emcfarlane
Copy link
Contributor

@emcfarlane emcfarlane commented Dec 4, 2023

To enable client retries, the http calls now operates directly on message payloads instead of relying solely on io.Writers. This refactor replaces write operations with send operations at message boundaries. It introduces a new interface, messagePayload, implemented by *bytes.Readers and *envelopes, enabling them to adopt sender operations.

Changes Made:

  • A new interface, messagePayload, implemented by *bytes.Reader and *envelope, allowing for adoption of sender operations.
  • A new interface, messageSender, implemented by *duplexHTTPCall, which acts on messagePayloads to send the message.
  • An adapter writeSender for http.ResponseWriter to implement messageSender via converting send payloads to write operations.
  • Replaced write operations with send operations at message boundaries for handling message payloads.

Objective:

The goal of this refactor is to facilitate client retries by directly manipulating message payloads rather than relying solely on io.Writers. The messagePayload interface is a readable, seekable and sized payload which allows it to be replayed.

This PR is part of the fix for #609 . It implements the groundwork for the handling of message payloads.

To enable client retries, the system now operates directly on message
payloads instead of relying solely on io.Writers. This refacotr replaces
write operations with send operations at message boundries. It
introducces a new inteface, messagePayload, implemented by
*bytes.Readers and *envelopes, enabling them to adopt sender operations.
duplex_http_call.go Outdated Show resolved Hide resolved
Copy link
Member

@jhump jhump left a comment

Choose a reason for hiding this comment

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

Nice! I left some comments, but they are mostly nits. LGTM

duplex_http_call.go Outdated Show resolved Hide resolved
duplex_http_call.go Outdated Show resolved Hide resolved
envelope.go Outdated Show resolved Hide resolved
envelope.go Outdated Show resolved Hide resolved
envelope.go Outdated Show resolved Hide resolved
@jhump jhump merged commit cb84690 into connectrpc:main Dec 6, 2023
8 checks passed
@emcfarlane emcfarlane deleted the ed/sender branch December 6, 2023 14:49
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.

2 participants