Skip to content
This repository has been archived by the owner on Mar 20, 2018. It is now read-only.

Fix bundling bug #114

Merged
merged 1 commit into from
Jun 29, 2016
Merged

Fix bundling bug #114

merged 1 commit into from
Jun 29, 2016

Conversation

geigerj
Copy link
Contributor

@geigerj geigerj commented Jun 28, 2016

Previously, one timer was associated with each Executor. This timer
was not reset when it fired, which meant that only a single request
could ever be triggered by a delay threshold.

This change switches to using one timer per bundle. This correctly
allows different bundles to fire under different timers.

cc: @tbetbetbe @jmuk
Fixes #113

@geigerj
Copy link
Contributor Author

geigerj commented Jun 28, 2016

Per offline discussion, I will investigate about adding more thorough tests for delay threshold-triggered bundling as a follow-up to this PR. See #115

@codecov-io
Copy link

codecov-io commented Jun 28, 2016

Current coverage is 97.16%

Merging #114 into master will not change coverage

@@             master       #114   diff @@
==========================================
  Files             8          8          
  Lines           599        599          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits            582        582          
  Misses           17         17          
  Partials          0          0          

Powered by Codecov. Last updated by a5e1944...d766e10

@tbetbetbe
Copy link
Contributor

LGTM

Previously, one timer was associated with each Executor. This timer
was not reset when it fired, which meant that only a single request
could ever be triggered by a delay threshold.

This change switches to using one timer per bundle. This correctly
allows different bundles to fire under different timers.
@bjwatson
Copy link
Contributor

LGTM

@geigerj geigerj merged commit 75fc5bd into googleapis:master Jun 29, 2016
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