(fix): refactor batch_event_processor to reset deadline after it passes. by thomaszurkan-optimizely · Pull Request #227 · optimizely/python-sdk · GitHub
Skip to content

(fix): refactor batch_event_processor to reset deadline after it passes. #227

Merged
thomaszurkan-optimizely merged 11 commits into
masterfrom
slightEPRefactor
Dec 14, 2019
Merged

(fix): refactor batch_event_processor to reset deadline after it passes. #227
thomaszurkan-optimizely merged 11 commits into
masterfrom
slightEPRefactor

Conversation

@thomaszurkan-optimizely

Copy link
Copy Markdown
Contributor

Also, hang on queue with timeout at flush interval

Summary

  • The deadline is not reset on the _run after the deadline has passed.
  • get can hang with a timeout. allow get to wait with timeout at flush interval

Test plan

Current tests should pass

Issues

…o, hang on queue with timeout at flush interval
@coveralls

Copy link
Copy Markdown

1 similar comment
@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.05%) to 97.598% when pulling a9a15ac on slightEPRefactor into 9fee8ff on master.

@coveralls

coveralls commented Dec 12, 2019

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.05%) to 97.599% when pulling 94f52e1 on slightEPRefactor into 9fee8ff on master.

@mikeproeng37

Copy link
Copy Markdown
Contributor

Fix looks fine to me. Do we need additional unit tests for this fix?

@mikeproeng37 mikeproeng37 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Lgtm

@aliabbasrizvi aliabbasrizvi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Synced with Tom and Mike on this. Changes make sense.

LGTM

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please change the description of the method.
Also Returns, valid time, can be float and integer. Need to revise documentation.

@msohailhussain msohailhussain left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm

@thomaszurkan-optimizely thomaszurkan-optimizely merged commit b156e21 into master Dec 14, 2019
@aliabbasrizvi aliabbasrizvi deleted the slightEPRefactor branch May 14, 2020 20:45
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.

5 participants