Skip to content
This repository has been archived by the owner on Aug 29, 2022. It is now read-only.

How to correctly track a Task's progress? #260

Closed
piercifani opened this issue Oct 10, 2018 · 11 comments
Closed

How to correctly track a Task's progress? #260

piercifani opened this issue Oct 10, 2018 · 11 comments
Milestone

Comments

@piercifani
Copy link
Contributor

Hi there!

I've written a URLSession extension that allows you to upload a file and wrap it in a task like this:

extension URLSession {

  public func uploadFile(with urlRequest: URLRequest, fileURL: URL) -> Task<APIClient.Response> {
    
    let deferred = Deferred<Task<APIClient.Response>.Result>()
    let uploadTask = self.uploadTask(with: urlRequest, fromFile: fileURL) { (data, response, error) in
      self.analyzeResponse(deferred: deferred, data: data, response: response, error: error)
    }
    uploadTask.resume()
    if #available(iOS 11.0, tvOS 11.0, watchOS 4.0, macOS 10.13, *) {
      return Task(deferred, progress: uploadTask.progress)
    } else {
      return Task(deferred, cancellation: { [weak uploadTask] in
        uploadTask?.cancel()
      })
    }
  }
}

As you see, I'm using the URLSessionUploadTask's progress to initialise the Task. After that, I'm using andThen to parse the JSON response from the server and more cleanup.

However, when observing the progress, the upload task is merely a fraction of the total progress, not corresponding to the much more heavier task that it represents.

How should we proceed in such cases? I realise this is more a NSProgress question than a Deferred question, but maybe someone can shed some light on this. You can see this behaviour in this commit of my project which is based on BNRDeferred

BTW: Really looking forward to version 4.0 of Deferred 💪

@piercifani
Copy link
Contributor Author

Whoops, just saw #259, I'll be following that instead

@piercifani
Copy link
Contributor Author

BTW: I updated my project to use the latest from master and I am seeing a difference in progress report, however the change is not exactly what I'm looking for: The fractionCompleted property is starting at 0.8 and then climbing all the way to 1.0, but it's still not yet correct IMHO. Maybe I'm doing something wrong?

@zwaldowski
Copy link
Contributor

zwaldowski commented Oct 22, 2018

The snippet you've provided reflects something I'd expect #259 to track correctly, as it was made while debugging a basically identical upload task (ping @DrewFitz?).

@piercifani
Copy link
Contributor Author

If you guys need a reproduceable project, you can clone
https://github.com/theleftbit/BSWFoundation/tree/deferred-4 and see this behavior in the testUpload test

@zwaldowski
Copy link
Contributor

Thanks, that's quite helpful!

Task's progress assumes that the "original" task is a larger piece of the job, and chaining done on it forms a smaller part. That assumption is non-ideal in your network request method. At a high level, I recommend performing one's andThen around the larger chunks of the job it would make sense to show up in the progress. However, I don't recommend changing what you have, as the functional chaining you've done is quite nice. 😃 I'll keep iterating on a better way to do this formula.

@piercifani
Copy link
Contributor Author

piercifani commented Oct 24, 2018

I have no idea on how to implement this using the current Deferred's stack, but I know how I'd like for it to work:

  • Tasks that are created without a discreet progress instance, are automatically assigned a Progress instance with a totalUnitCount of 1
  • Tasks created with a progress instance can use totalUnitCount as they want.
  • Every andThen would just create a new Progress whose totalUnitCount is a sum of the previous count of all child processes

This way, when a simple Task is created with no Progress provided, the initialiser will add one with NSProgress.progressWithTotalUnitCount(1); and another complex Task (like the upload one) will provide a progress with, say, a 50 count. The andThen that wraps both will essentially have a 51 total unit count. This will make the progress tracking very easy in almost all scenarios IMHO.

I don't know if this rambling makes much sense but thanks for hearing me out!

@piercifani
Copy link
Contributor Author

Any news on this? I'm very close to shipping the final version of the app to the client and I'd like to correctly do this if possible, otherwise I'd have to workaround this (temporary) limitation using NSNotification

@zwaldowski
Copy link
Contributor

I haven't been able to return to it until very recently. I hope to have something today, and I'm sorry if that's too late for you.

@piercifani
Copy link
Contributor Author

No worries Zach, I completely understand! Thanks a lot for your effort

@zwaldowski
Copy link
Contributor

#265 should get you out of the woods. I've filed some secondary improvements into #267 before I tag a next beta, but please give it a whirl. Thank you for your patience!

@piercifani
Copy link
Contributor Author

No @zwaldowski, thank YOU!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants