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

3scale batching policy: fix batching period assignment from config #710

Merged
merged 5 commits into from
May 15, 2018

Conversation

davidor
Copy link
Contributor

@davidor davidor commented May 15, 2018

This PR fixes a bug in the 3scale batching policy. It didn't assign the batching period according to the given configuration.
While at it, this PR also adds some debugging logs and tests that helped to detect this issue.

davidor and others added 4 commits May 15, 2018 14:32
…ng the test

If the report timer runs during the tests, it will alter the results.
We set a batching period high enough to prevent this from happening, but
as we cannot guarantee it, we need to check that it didn't happen.
@davidor davidor changed the title Fix batching param batching policy 3scale batching policy: fix batching period assignment from config May 15, 2018
@davidor davidor requested a review from mikz May 15, 2018 12:59
@@ -255,6 +255,7 @@ push $res, "Host: one";
$res
--- no_error_log
[error]
3scale batcher report timer got
Copy link
Contributor

Choose a reason for hiding this comment

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

🕺

Copy link
Contributor

@mikz mikz left a comment

Choose a reason for hiding this comment

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

👍 🥇

@davidor davidor merged commit 4e54345 into master May 15, 2018
@davidor davidor deleted the fix-batching-param-batching-policy branch May 15, 2018 13:30
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