Skip to content
This repository has been archived by the owner on Sep 15, 2022. It is now read-only.

retry: Allow setup of buffer for request body if GetBody property… #55

Merged

Conversation

robbieheywood
Copy link
Contributor

Changes

  • Add the ability for the retry tripperware to setup buffers that can be used to read the request body multiple times if it doesn't have a GetBody property.
  • Add an option to allow turning the above behaviour on.
  • Testing updated

Verification

Retry tripperware integration tests added to and passing

Copy link
Contributor

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

Hey, so recently we add this to kEdge, wonder if we can do similar here without extra option?

improbable-eng/kedge#152

What do you think?

@robbieheywood
Copy link
Contributor Author

Hey, so recently we add this to kEdge, wonder if we can do similar here without extra option?

improbable-eng/kedge#152

What do you think?

Ah interesting, thanks for pointing out! I added the option to be conservative (it's opt-in) but would be happy to just make this the default if you think that's better?

@bwplotka
Copy link
Contributor

Relying on GetBody might be misleading that's why we added the resetable, buffered reader (:
We found it quite stable, so I would try to make it a default, especially if you want to do it only for certain edge cases like no GetBody - but up to you.

@robbieheywood robbieheywood force-pushed the retry-tripperware-body-buffering branch from 1b87573 to b1f1be3 Compare June 25, 2019 22:52
retry/get_body_go17.go Outdated Show resolved Hide resolved
retry/tripperware.go Outdated Show resolved Hide resolved
retry/tripperware.go Outdated Show resolved Hide resolved
retry/tripperware.go Outdated Show resolved Hide resolved
retry/integration_test.go Show resolved Hide resolved
retry/tripperware.go Show resolved Hide resolved
@easyCZ easyCZ changed the title retry: Allow setup of buffer for request body if GetBody property isn't present retry: Allow setup of buffer for request body if GetBody property… Jun 26, 2019
@easyCZ easyCZ merged commit a661e9d into improbable-eng:master Jun 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants