balena-io / balena-sdk

The SDK to make balena powered JavaScript applications.
https://www.balena.io/
Apache License 2.0
145 stars 46 forks source link

allow `.remove()` and others to accept arrays as arguments. #164

Open shaunmulligan opened 8 years ago

shaunmulligan commented 8 years ago

would be cool if functions like resin.models.device.remove() could take an array of uuids as an argument. It would make batch deleting and other actions much easier :smile:

jviotti commented 8 years ago

I wonder how to correctly deal with deletion failures in any of the devices.

For example, consider we have devices aaaa, bbbb and cccc. We trigger a batch deletion as:

resin.models.device.remove([ 'aaaa', 'bbbb', 'cccc' ])

But bbbb fails. Would you expect the promise to be rejected?

I can think of the following approaches:

  1. Reject the whole promise if a single device can't be removed (notice that some devices could have been removed successfully already).
  2. Fulfil the promise swallowing any error.
  3. Return the ones that failed but still resolve the promise.

Option 3 looks reasonable to me.

shaunmulligan commented 8 years ago

In my usecase I made it reject unless all of them deleted successfully, but its definitely not the correct approach. What are the implications of rejecting the whole promise, is there a way an error can be returned indicating which devices weren't successfully deleted?

Page- commented 8 years ago

I wonder whether it would be better to just provide examples of doing this via something like Promise.map([ 'aaaa', 'bbbb', 'cccc' ], resin.models.device.remove)?

jviotti commented 8 years ago

@Page- Are there any advantages on doing a batch removal with a single pine request?

Page- commented 8 years ago

It will be quicker, and will also be all or nothing

jviotti commented 8 years ago

Ah nice, so I guess we can rely on pine being smart enough to remove all or nothing.