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

Implement task polling composable #13158

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

ozer550
Copy link
Member

@ozer550 ozer550 commented Mar 5, 2025

Summary

  • Implement task polling composable
  • Use the composable in the EditDeviceSyncSchedule component, fixing a bug that caused multiple setIntervals to poll the same queue at the same time.

Before:

Compartir.pantalla.-.2025-03-10.16_10_18.mp4

After:

Compartir.pantalla.-.2025-03-10.16_13_51.mp4

References

Closes #13093

Reviewer guidance

  • Go to Device > facilities > [facility] > manage sync facilities
  • Add new / edit sync schedule and set hourly frequency
  • While the sync task is running (the edit schedule inputs are disabled) keep the edit sync schedule modal open for some time
  • While you are in the edit sync schedule modal, open your network tab and check that requests behave normally and that they dont increment over time.

@ozer550 ozer550 changed the title add skeleton for task pooling composable add skeleton for task polling composable Mar 6, 2025
@AlexVelezLl AlexVelezLl force-pushed the create-task-polling-composable branch from fa463dd to ad671c0 Compare March 10, 2025 20:52
@AlexVelezLl AlexVelezLl force-pushed the create-task-polling-composable branch from ad671c0 to be6a9ef Compare March 10, 2025 20:58
@AlexVelezLl AlexVelezLl changed the title add skeleton for task polling composable Implement task polling composable Mar 10, 2025
@AlexVelezLl AlexVelezLl marked this pull request as ready for review March 10, 2025 21:08
Copy link
Member

@rtibbles rtibbles left a comment

Choose a reason for hiding this comment

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

Not seeing any concerns with the implementation, but it has fallen a little short of the issue - at the very least I would have expected to see the task polling that is happening here: https://github.com/learningequality/kolibri/blob/develop/packages/kolibri-common/components/SyncSchedule/ManageSyncSchedule.vue to be replaced by this composable too (as switching between this component and the edited Edit component were the main causes of the bug).

@rtibbles rtibbles self-assigned this Mar 11, 2025
@ozer550 ozer550 requested a review from rtibbles March 12, 2025 11:50
Copy link
Member

@rtibbles rtibbles left a comment

Choose a reason for hiding this comment

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

This is looking as I'd expect - @pcenov @radinamatic if you could check the sync scheduling workflows on this, and ensure we're not seeing any regressions in viewing, creating, updating scheduled syncs, that would be great.

@rtibbles rtibbles dismissed their stale review March 12, 2025 23:51

Comments addressed.

@pcenov
Copy link
Member

pcenov commented Mar 17, 2025

Hi @ozer550 the only regression I am observing is that if after having added a sync schedule I click the "Edit" button, then I'm not seeing the correct value in the "Frequency" drop-down and there are errors in the console. If I click "Cancel" and then click "Edit" again, then I am seeing the correct value:

frequency.mp4

@ozer550
Copy link
Member Author

ozer550 commented Mar 18, 2025

Hi @pcenov! I am unable to reproduce this.

regression.mp4

@pcenov
Copy link
Member

pcenov commented Mar 18, 2025

Hi @ozer550 - could you try it out by creating a sync schedule for an actual device, not KDP?

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.

Create a composable and update task polling
4 participants