Skip to content

Add more control over BatchEventProcessor. #319

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jul 12, 2019
Merged

Add more control over BatchEventProcessor. #319

merged 3 commits into from
Jul 12, 2019

Conversation

mikecdavis
Copy link
Contributor

Summary

  • Add flush method to BatchEventProcessor.
  • Allow BatchEventProcessor to be restarted.
  • Add option to build BatchEventProcessor without starting the processing thread.

@coveralls
Copy link

coveralls commented Jul 11, 2019 •
edited
Loading

Pull Request Test Coverage Report for Build 1139

  • 13 of 13 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.03%) to 89.624%
Totals Coverage Status
Change from base Build 1138: 0.03%
Covered Lines: 3576
Relevant Lines: 3990
💛 - Coveralls

mjc1283
mjc1283 reviewed Jul 12, 2019
View reviewed changes
core-api/src/main/java/com/optimizely/ab/event/BatchEventProcessor.java Outdated
if (item == FLUSH_SIGNAL) {
logger.debug("Received flush signal.");
flush();
break;
Copy link

Choose a reason for hiding this comment

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

Is it correct to break here? Then the event consumer will exit, and no more events will be consumed, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yikes. Yeah should be a continue; 🤦‍♂

@mikecdavis mikecdavis force-pushed the mikecdavis/OASIS-5043-lifecycle branch from faa4c53 to 56c3cdc Compare July 12, 2019 15:43
@mikecdavis mikecdavis requested a review from mjc1283 July 12, 2019 15:46
aliabbasrizvi
aliabbasrizvi approved these changes Jul 12, 2019
View reviewed changes
Copy link
Contributor

@aliabbasrizvi aliabbasrizvi left a comment

Choose a reason for hiding this comment

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

LGTM

@mikecdavis mikecdavis merged commit d19cdcf into master Jul 12, 2019
@mikecdavis mikecdavis deleted the mikecdavis/OASIS-5043-lifecycle branch July 12, 2019 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
2 more reviewers

@mjc1283 mjc1283 mjc1283 approved these changes

@aliabbasrizvi aliabbasrizvi aliabbasrizvi approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mikecdavis @coveralls @mjc1283 @aliabbasrizvi