-
Notifications
You must be signed in to change notification settings - Fork 39
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
(fix):potential fix for multi scheduling of event work #268
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, nice fix!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
job dedupe logic lgtm
@@ -101,8 +101,10 @@ void reschedule(@NonNull Context context, @NonNull Intent broadcastIntent, @NonN | |||
// with wifi the service will be rescheduled on the interval. | |||
// Wifi connection state changes all the time and starting services is expensive | |||
// so it's important to only do this if we have stored events. | |||
ServiceScheduler.startService(context, EventIntentService.JOB_ID, eventServiceIntent); | |||
logger.info("Preemptively flushing events since wifi became available"); | |||
if (android.os.Build.VERSION.SDK_INT < Build.VERSION_CODES.O) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not schedule on Android O?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm adding a comment. But, it is because in Android O and higher, we use the job scheduler and make the job persistent. this means that it will restart itself.
* potential fix for multi scheduling of event work * add a comment why not restarting in event rescheduler
Summary
Issues