Add support for canceling a download if the expected time to complete… #357
Conversation
… is less than provided value.
| if (self.sessionTaskEndTime > 0) { | ||
| return 0; | ||
| } | ||
| CFTimeInterval taskLength = CACurrentMediaTime() - self.sessionTaskStartTime; |
There was a problem hiding this comment.
Maybe call this elapsedTime?
| cache and used to resume the download if the same URL is attempted to be downloaded in the future. | ||
| */ | ||
|
|
||
| - (void)cancelTaskWithUUID:(nonnull NSUUID *)UUID ifExpectedToExceedTimeInterval:(NSTimeInterval)timeToExceed storeResumeData:(BOOL)storeResumeData; |
There was a problem hiding this comment.
This time interval is assumed to be relative to "now" when it's provided but won't be evaluated until some (small) time in the future.
Might be better (clearer and more predictable) for this to take an absolute "deadline" time beyond which any longer-running tasks are cancelled.
I know precision here is sloppy because it's all based on estimates, but it still might be a better API overall.
There was a problem hiding this comment.
I can't decide on this. My thought is that if you're using this API, you want to cancel something unless it's got just a bit left to download.
Here's the scenario I imagine:
- At time 0 you ask to cancel a download if it doesn't complete in the next 100ms
- At time 100ms, the cancel request is serviced and the image has 50ms left to download
In that case, what do we think the caller would want? If we are provided an absolute time, the image will continue to download until there is 50ms left and then be canceled. If we provide a relative time, the image will be downloaded until there is 50ms left and be allowed to continue.
I think I would rather specify this as an interval because I care about how much time is left when the cancelation occurs, not when I call the method?
There was a problem hiding this comment.
Also, any better ideas on naming here? It's currently very atrocious :D
There was a problem hiding this comment.
I see your point. On one hand, we're "asking" PINRemoteImage to save us some network bandwidth based on best effort measurements. On the other, we believe the caller has a specific plan of action, and part of that plan involves dropping in-flight network requests.
Relatedly, would it be helpful to think of this as a "suspend" operation rather than a "cancel" operation?
| @@ -113,4 +115,21 @@ - (nonnull PINRemoteImageManagerResult *)imageResultWithImage:(nullable PINImage | |||
| bytesSavedByResuming:bytesSavedByResuming]; | |||
| } | |||
|
|
|||
| - (CFTimeInterval)expectedSecondsLeftToComplete | |||
There was a problem hiding this comment.
CFTimeInterval is always in seconds. Maybe expectedTimeLeftToComplete?
There was a problem hiding this comment.
Ooh, way better (I was relying on this review to fix my horrible naming)
Generated by 🚫 Danger |
|
Closing in favor of #364 |
… is less than provided value.