Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TIMOB-17327]: Add ceiling and floor to httpClient's onDataStream progress #6424

Merged
merged 7 commits into from Dec 5, 2014

Conversation

jonalter
Copy link
Contributor

@jonalter jonalter commented Dec 3, 2014

@ingo
Copy link
Contributor

ingo commented Dec 3, 2014

Test comment.

@jonalter
Copy link
Contributor Author

jonalter commented Dec 3, 2014

Test response

@ingo
Copy link
Contributor

ingo commented Dec 3, 2014

Waiting for CLA hook to kick in. Think it takes 5 minutes.

@jonalter
Copy link
Contributor Author

jonalter commented Dec 3, 2014

👍

@ingo
Copy link
Contributor

ingo commented Dec 3, 2014

And trying again.

@ingo
Copy link
Contributor

ingo commented Dec 4, 2014

Attention: The contributor has signed the CLA

@ingo
Copy link
Contributor

ingo commented Dec 4, 2014

Attention: The contributor has signed the CLA

double currentProgress = ((double)progress)/totalLength;
// cap progress to 1 and floor at 0
if (currentProgress > 1 || currentProgress < 0) {
currentProgress = 1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why it's default to 1 when the calculated progress is invalid in "onsendstream", but default to -1 in "ondatastream"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In ondatastream we do this because sometimes we do not get content-length and it creates crazy values. in onsendstream we always know what the size is, so just adding this as a check. Per Vishal

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No matter why we fall into this invalid value situation, it is still invalid. So in my opinion, once the value is invalid, we set it to the NetworkModule.PROGRESS_UNKNOWN constant.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After talking to Vishal, we are going to remove the check for 0-1 in upload progress. It should never be outside that range, but if it is, it is a bug and we want to see it.

@bhatfield
Copy link
Contributor

Reviewed and ran validate.

DR passed.

@pingwang2011
Copy link
Contributor

CR passed.
Ran KS->XHR passed.
Accepted

pingwang2011 added a commit that referenced this pull request Dec 5, 2014
[TIMOB-17327]: Add ceiling and floor to httpClient's onDataStream progress
@pingwang2011 pingwang2011 merged commit 60ec73f into tidev:master Dec 5, 2014
@jonalter jonalter deleted the TIMOB-17327 branch December 5, 2014 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants