MotionModel icon indicating copy to clipboard operation
MotionModel copied to clipboard

Feature Request: Different notification for Bulk Update complete

Open cognitiveflux opened this issue 10 years ago • 5 comments

There doesn't appear to be a hook or notification if using the bulk_update {} block. Perhaps there's a recommended way to do this that's not documented? Otherwise, I'm willing to give the implementation a shot if you define any restrictions or caveats for implementation.

I'm proposing something like a MotionModelDataDidCompleteBulkUpdateNotification, thoughts?

cognitiveflux avatar Oct 31 '14 01:10 cognitiveflux

This would be extremely helpful. I've found that dribbling out updates in groups of 10 or something is a very effective way to keep the UI alive. Consider:

bulk_update(notify_interval: 10) {
  # some stuff
}

That way, you could issue MotionModelProgressNotification and at the end MotionModelDataDidCompleteBulkUpdateNotification. WDYT?

sxross avatar Oct 31 '14 17:10 sxross

I've noticed that record sets of 100 load incredibly quick, but I agree and think it's best to allow the frequency to be defined as in your example so people can fine tune to their needs and data types. I'll familiarize myself with how MotionModel handles notifications.

A couple requirement specs/clarifications:

  • If the notify_interval is not provided, default to only sending the completion?
  • What is the significance of 'completion': does it mean that the update was performed but perhaps not all records were created? Or will MotionModel raise or notify if an individual create fails? Are there any guarantees of complete success or complete failure?

cognitiveflux avatar Oct 31 '14 18:10 cognitiveflux

I would default to completion (i.e., end of block) if interval is not set. The latency is typically not in MotionModel, but in remote services like Firebase or (in the case of my current app) Meteor. They try to do as-available data supply, which can be chunked. I've found that the app "looks" like it's hanging on startup if I don't show something every so often.

When an interval is provided, completion is defined as "all the rest are updated". This would only be relevant if the supplier sent a "ready" notification, but it still useful. I think this deserves some thought, as timeout might be another consideration for data suppliers that throttle.

sxross avatar Oct 31 '14 19:10 sxross

Are you thinking of usage like the following?:

async_remote_request do |response|
   bulk_update do
     response.body.each { |r| Model.create(r) }
   end
end

If you're doing the bulk update after the response is received the timeout wouldn't be a concern because each chunck will perform a bulk_update, correct?

With the new notifications, the UI can be updated each time the chunk is received, instead of when each record is created (currently can't use bulk_update since no notification is fired, so using a normal create for every record fires a notification). Using the above example, I'm not sure what benefit the notify_interval option would provide.

If you were thinking of wrapping the remote request within the bulk_update block then there are a myriad of concerns that come up and it might be best to not mix the task of handling remote data sources within the context of MotionModel handling local data. Thoughts?

cognitiveflux avatar Oct 31 '14 20:10 cognitiveflux

I misspoke. Of course you're right about not wrapping request/response inside one of these blocks. The request/response would (or should) run asynchronously, and the block would almost immediately complete, triggering the completion notification. The use case for :notify_interval I was referring to was where object creation was complicated and possibly time-consuming. Perhaps "interval" is poor semantics. Maybe something like:

# Option 1: Add lots of syntactic sugar
bulk_update(and_progress_notify: every(10).items) {
  # some stuff
}

# Option 2: More terse, but still shows intent
bulk_update(notification_item_frequency: 10) {
  # some stuff
}

I don't want to overthink this. But I do think completion notification will be used. I know I have already done both that and intervals, but not backported them to MotionModel (my bad).

sxross avatar Nov 01 '14 06:11 sxross