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

fix: request time contains RequestQueue blocking time. #140

Merged
merged 1 commit into from
Jan 10, 2018

Conversation

dezng
Copy link
Contributor

@dezng dezng commented Jan 9, 2018

No description provided.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, 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.
  • If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot. The email used to register you as an authorized contributor must be the email used for the Git commit.
  • In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

@dezng
Copy link
Contributor Author

dezng commented Jan 9, 2018

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@jpd236 jpd236 added this to the 1.1.1 milestone Jan 10, 2018
@jpd236
Copy link
Collaborator

jpd236 commented Jan 10, 2018

Interesting find... seems to have been this way since 1.0.0 at least. Doesn't seem useful in its current form because take() will block indefinitely until a request actually gets queued.

Note though that despite this, the actual network time is also available via NetworkResponse#networkTimeMs, and I don't think that has an issue like this, though it's only present for requests that actually get a network response.

@uhager I know you're using this field - any concerns about this change from your end? If not we should probably take it (and perhaps update the Javadoc for VolleyError#getNetworkTimeMs which is currently non-existent).

@uhager
Copy link
Contributor

uhager commented Jan 10, 2018

I don't think we have ever really looked at the network time for errors, only for responses.
This change looks good to me.

@jpd236 jpd236 merged commit b89dfbd into google:master Jan 10, 2018
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.

4 participants