http
http copied to clipboard
Add onSendProgress callback
This fixes the upload part of https://github.com/dart-lang/http/issues/465
I had also tried to include the download progress, but that did not work as expected. And it is why easier to do this while consuming the response stream.
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).
:memo: Please visit https://cla.developers.google.com/ to sign.
Once you've signed (or fixed any issues), please reply here with @googlebot I signed it!
and we'll verify it.
What to do if you already signed the CLA
Individual signers
- It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.
Corporate signers
- Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
- The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
- The email used to register you as an authorized contributor must also be attached to your GitHub account.
ℹ️ Googlers: Go here for more info.
@googlebot I signed it!
Hope this gets merged, it would really help
@tlueder @jasonroland @piotrsed any updates on this?
Hi, I can rebase this. But without a clear path forward I don't see the point.
Currently I just use this plugin from my repro via dependency override, works great.
@brianquinlan @devoncarew @natebosch
Please fix merge conflicts!
@kevmoo done
This looks like a breaking change, and we have been avoiding breaking API changes. This is only breaking for a subset of users, but I don't know what proportion.
We are currently getting through a major version bump without too much difficulty, but that was a version bump without breaking API changes. cc @brianquinlan for thoughts on the practicality of a breaking API change and another version bump.
This is a API change for sure but I would argue against a breaking one. This adds a new optional feature. And should not break anything for anyone not using it.
Can you elaborate what you mean by "This is only breaking for a subset of users"? Maybe I can change the code to make sure it won't affect them.
This is breaking for classes with overrides of the changed methods.
Of course, I haven't even thought about that. Thanks for the information.
We are currently getting through a major version bump without too much difficulty, but that was a version bump without breaking API changes. cc @brianquinlan for thoughts on the practicality of a breaking API change and another version bump.
- As you said, this is definitely a breaking change (in particular it will definitely break at least
package:cronet_http
andpackage:cupertino_http
). - I wonder if there is a way to implement this without breaking the API? For example, we could define a new class like:
interface class ProgressTracker {
void onSendProgress(int loaded, int total);
}
class ProgressTrackingRequest extends StreamedRequest implements ProgressTracker {
...
}
And send
implementations could check if the Request
implements that class and, if so, could update the progress e.g.
if (request is ProgressTracker) {
request.onSendProcess(...);
}
That's a bit ugly.
- I'm not sure why tracking upload progress is more important than tracking download progress. If we are going to make a breaking change then we should do both so we don't have to update the major version just to add the obvious complementary feature.
- I'd be fine with another version bump but maybe we should wait a few months and accumulate all the breaking changes into one release? Is there an easy way to do this? Like have a 2.0 branch or something?
- Understood, and thanks for your input I was not aware of that.
- I'd rather wait for a version bump instead of a "quick fix". But in the end, I don't really care so much about the how this is implemented, because I don't know enough about dart best practice and what side effects this can have. I just need the progress data and if this is done a different way? I'm happy to update my apps if that means I can get rid of the dependency override and rebasing my fork.
- It is not that tracking the download is not important, but in contrast to the upload it can be done easily on the consuming side as all the needed data is already available. Just not as callback function. And when I wrote this, 2 years ago, I was not able to do this and my attempts felt just wrong. So I left it at that, but any hints in the right direction are welcome.