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

Update grpc #1031

Closed
wants to merge 1 commit into from
Closed

Update grpc #1031

wants to merge 1 commit into from

Conversation

ocelotl
Copy link
Contributor

@ocelotl ocelotl commented Aug 21, 2020

Update grpc to a more recent version.

@ocelotl ocelotl requested a review from a team August 21, 2020 02:44
@ocelotl ocelotl self-assigned this Aug 21, 2020
def _verify_error_records(
self, method, status_code=grpc.StatusCode.UNAVAILABLE
):
# Updating grpc to 1.31.0 makes the status codes for two test cases
Copy link
Contributor

Choose a reason for hiding this comment

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

i guess grpc 1.31.0 changed something in how connections are handled. the reason why the tests are now returning a different status code is that self.channel was not properly closed on tearDown (see #1027). the left over connection/channel of the previous test is then causing that the current test cannot connect to the test server which in turn leads to the status code UNAVAILABLE.
if just the failing test is run it should actually be green, or if failing test is removed the next test will suddenly fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, @mariojonke. After the changes added in #1027 to properly close the channel, I can run the test cases with tox -e py38-test-instrumentation-grpc with grpcio==1.31.0. This PR does not seem necessary anymore, closing it.

@ocelotl ocelotl closed this Aug 31, 2020
@ocelotl ocelotl deleted the issue_1030 branch August 31, 2020 16:59
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.

2 participants