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: extend supported gRPC version range to 1.8 #630

Merged
merged 1 commit into from
Jan 4, 2018

Conversation

kjin
Copy link
Contributor

@kjin kjin commented Jan 4, 2018

  • Bump supported gRPC version range to 1.8
  • Change gRPC fixture versions to be ~ instead of ^ (to fix CI failure after gRPC 1.8 was released)
  • datastore test: Change checked error code from HTTP 401 (unauthorized) to gRPC 16 (unauthenticated) -- reflects change in gRPC 1.8
  • For convenince, add npm run script npm script

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 4, 2018
@stevenaldinger
Copy link

stevenaldinger commented Jan 4, 2018

heads up, we needed this update and gave it a whirl (you had opened the PR 22 minutes prior) and we got "unknown errors" in our grpc client suddenly. I'm at work right now and all proprietary unfortunately but things went from simply not showing up in the cloud trace UI to actually breaking the client itself. i was already using a fork with a semver patch (one line of code change in the loader) and merged this in/rebuilt and had no luck. wish i could help more, I'm not a type scripter or git expert so very well could be user error, I see the CI tests passed.

i use grpc 1.8 outside of work too and will try to provide more info if its reproducible, I'm happy to share my personal client

@kjin
Copy link
Contributor Author

kjin commented Jan 4, 2018

@stevenaldinger This PR shouldn't result in any additional behavior changes because JavaScript changes in gRPC 1.8 seem to be minimal. Do you know if this is an issue with gRPC itself? It's possible considering the unknown errors you got -- to my knowledge the Trace Agent isn't emitting or changing gRPC errors.

@stevenaldinger
Copy link

Honestly the code base I tried using it with is such a mess I felt guilty even posting that. In about 3 hours I'll reply back here when I can try it with my setup at home. Very very believable it'd be something outside of your control here, especially with what you just said.

@kjin kjin merged commit 3b1cd24 into googleapis:master Jan 4, 2018
@stevenaldinger
Copy link

Had reverted to the current stable version on npm and got the same error after a while, safe to say its not you. Thanks a lot for this PR! Can't wait to use it

@kjin
Copy link
Contributor Author

kjin commented Jan 4, 2018 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants