Skip to content
This repository was archived by the owner on Sep 26, 2023. It is now read-only.

Conversation

@igorbernstein2
Copy link
Contributor

@igorbernstein2 igorbernstein2 commented Mar 12, 2018

I apologize for the noise, I thought I could use this functionality for batching in googleapis/google-cloud-java#3026, but turns out I was wrong and had to implement a custom attempt callable regardless.

I still think this functionality (along with response suppression) can be useful for spanner, but I figured I'd revert it until it is actually needed.

@igorbernstein2
Copy link
Contributor Author

@garrettjonesgoogle please merge if you agree. If you think it's useful to keep, please close this PR

@codecov-io
Copy link

Codecov Report

Merging #495 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff            @@
##             master    #495   +/-   ##
========================================
  Coverage      70.5%   70.5%           
  Complexity      810     810           
========================================
  Files           165     165           
  Lines          3773    3773           
  Branches        289     289           
========================================
  Hits           2660    2660           
  Misses          990     990           
  Partials        123     123
Impacted Files Coverage Δ Complexity Δ
...i/gax/retrying/SimpleStreamResumptionStrategy.java 14.28% <ø> (ø) 1 <0> (ø) ⬇️
...le/api/gax/rpc/ServerStreamingAttemptCallable.java 86.79% <100%> (ø) 17 <0> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b4eae66...5e17c36. Read the comment docs.

@garrettjonesgoogle
Copy link
Member

So there is a renaming in the original PR. I'd prefer not to flip-flop on naming. Also, there is very little overhead in this feature. I think we should just keep it.

@igorbernstein2 igorbernstein2 deleted the revert-resumption-response-processing branch March 18, 2018 06:28
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.

3 participants