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

Provide a StateFlow for sync status #554

Closed
Tracked by #524
jingtang10 opened this issue Jun 16, 2021 · 8 comments · Fixed by #648
Closed
Tracked by #524

Provide a StateFlow for sync status #554

jingtang10 opened this issue Jun 16, 2021 · 8 comments · Fixed by #648
Assignees
Labels
type:enhancement New feature or request

Comments

@jingtang10
Copy link
Collaborator

Is your feature request related to a problem? Please describe.
We should provide developers access to the status of the sync jobs in order to update the UI.

Describe the solution you'd like
We could use a StateFlow to wrap the state of the work manager

https://developer.android.com/kotlin/flow/stateflow-and-sharedflow

Describe alternatives you've considered
Use LiveData: https://developer.android.com/codelabs/android-workmanager#7

@yigit made the recommendation that StateFlow is the new API and we should use it. This article talks about hwo to replace LiveData with StateFlow: https://betterprogramming.pub/android-data-binding-with-mvvm-using-stateflow-and-viewmodel-bce04d0b9bc4

Additional context
NA

Would you like to work on the issue?
NA

@jingtang10 jingtang10 added the type:enhancement New feature or request label Jun 16, 2021
@jingtang10 jingtang10 added this to To do in FHIR engine library via automation Jun 16, 2021
@Manikant25
Copy link
Contributor

@jingtang10 Can I take this issue?

@jingtang10
Copy link
Collaborator Author

@jingtang10 Can I take this issue?

Really sorry but Maimoona was already working on this. Thanks for your enthusiasm please keep an eye on the repo and we can assign you something else.

@Manikant25
Copy link
Contributor

No problem

@Tarun-Bhardwaj
Copy link

@maimoonak , is there an ETA by when this issue can be closed? It is blocking #605 which is a high priority for Q3 v0 release.

@Tarun-Bhardwaj
Copy link

@maimoonak , is there an ETA by when this issue can be closed? It is blocking #605 which is a high priority for Q3 v0 release.

@maimoonak , any update on the ETA by when this issue can get closed?

@maimoonak
Copy link
Collaborator

There are multiple options to run polling with flows.

  • channelFlow : allows elements to be produced running in a different context or concurrently (would be helpful when we need to run the sync for resources concurrently and wait until whole sync completes itself, would be needed for sync-workers)
  • stateFlow, sharedFlow, tickerFlow (obsolete): For poller we do not need any custom handling or complex flow, so we just implement simple channelFlow.

interface SyncJob -> to allow multiple or different polling strategies
class SyncJobImpl -> first basic polling implementation with given delay

19b2e26 very basic polling with test

@maimoonak
Copy link
Collaborator

@Tarun-Bhardwaj I tried my best to finish this by this week. However, it looks like in initial discussions we had missed the power that WorkManager gives. I have pushed the code with tests on branch 554_flowapi for onetime sync with flow API, and now trying to see if we can use WorkManger with flow. The WorkManger creates the instances itself and provides a very limited access to running service data later, but we can not avoid it because we want service run even if app is closed, which is provided by WorkManager only.
@jingtang10 need your help with Periodic Sync with WManger and further discussions on this

@maimoonak
Copy link
Collaborator

@jingtang10 So based on few things missing into our last discussion I have added a complete workflow which implements one-time-sync and polling as well. Both use flow however, since workmanager does not work on flows, I have a doubt on the approach. I also have made the code a little verbose by adding questions and extra comments which would be deleted before making a formal PR. However, I have much more clarity and answers to queries raised in previous discussion.
https://github.com/google/android-fhir/pull/648/files this PR reflects the work. Kindly review and then we can discuss it further

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:enhancement New feature or request
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants