Skip to content

Activity lifecycle: Stop only the services on stop#111

Draft
manuq wants to merge 2 commits into
developfrom
T34626-stop-services-only
Draft

Activity lifecycle: Stop only the services on stop#111
manuq wants to merge 2 commits into
developfrom
T34626-stop-services-only

Conversation

@manuq
Copy link
Copy Markdown

@manuq manuq commented Mar 20, 2023

On stop: stop only the Kolibri services by directly calling ServicesPlugin.STOP.

On resume: if the Kolibri bus is already started, start the services. If not, transition the bus to the START state which should start the services too.

https://phabricator.endlessm.com/T34626

@manuq manuq requested a review from dylanmccall March 20, 2023 19:54
On stop: stop only the Kolibri services by directly calling
ServicesPlugin.STOP.

On resume: if the Kolibri bus is already started, start the services. If
not, transition the bus to the START state which should start the
services too.
@manuq manuq force-pushed the T34626-stop-services-only branch from 4501f22 to 8f2be4a Compare March 20, 2023 20:00
Comment on lines +56 to +58
def start_services(self):
self._services.START()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It's possible for an activity's onStart() callback to run multiple times in between calls to onStop(), for example if the activity is paused but not stopped. Looking at ServicesPlugin source, it appears the iceqube engine is going to be created from scratch each time the START() function runs: https://github.com/learningequality/kolibri/blob/bbd63a9c1f0371dc5009f59375c43aec26092610/kolibri/utils/server.py#L255-L267.

So, I think we should add a way to keep track of whether the services plugin has been stopped or started so we aren't doing it needlessly. Perhaps add our own PausableServicesPlugin which keeps track of that state?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for the feedback! I was able not only to skip the START when it wasn't stopped before, but also to partially start after stopped. To prevent calling schedule_ping() and schedule_vacuum() multiple times. See the fixup commit.

@manuq manuq force-pushed the T34626-stop-services-only branch from 303e131 to 25e1f7e Compare March 21, 2023 15:17
@manuq manuq marked this pull request as draft March 21, 2023 17:50
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