Skip to content

Conversation

xmakro
Copy link
Contributor

@xmakro xmakro commented Dec 21, 2024

Otherwise the gRPC client will stay connected past the shutdown

Fixes #4429

@xmakro xmakro requested a review from a team as a code owner December 21, 2024 17:46
Copy link

linux-foundation-easycla bot commented Dec 21, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@xrmx
Copy link
Contributor

xrmx commented Dec 23, 2024

@xmakro Thanks for the PR, please add a CHANGELOG entry. Can this be tested?

@xmakro
Copy link
Contributor Author

xmakro commented Dec 23, 2024

I added the change log. I don't see a good way to test the disconnect, since it is the gRPC channel that gets disconnected, but these channels are not exposed on the gRPC server side api.

@rjduffner
Copy link
Contributor

@xmakro, do you have time to look into these failures? Or do you mind if I branch and take a stab?

@xmakro
Copy link
Contributor Author

xmakro commented Feb 19, 2025

The checks report a ruff error. I synced this branch to head, updated ruff and ran the pre-commit and cannot reproduce this error. I'm happy if you can fix this by branching.

% git pull
remote: Enumerating objects: 3, done.
remote: Counting objects: 100% (3/3), done.
remote: Compressing objects: 100% (3/3), done.
remote: Total 3 (delta 0), reused 2 (delta 0), pack-reused 0 (from 0)
Unpacking objects: 100% (3/3), 22.66 KiB | 429.00 KiB/s, done.
From https://github.com/xmakro/opentelemetry-python
   d71bd270..f90d38fd  patch-1    -> origin/patch-1
Already up to date.

% pre-commit run --color=always --all-files
ruff.....................................................................Passed
ruff-format..............................................................Passed

@rjduffner
Copy link
Contributor

rjduffner commented Feb 19, 2025

@xmakro, I was able to reproduce what the linter here was failing on.

xmakro#1

Maybe your ruff settings are set a bit different?

@rjduffner
Copy link
Contributor

@xmakro just checking in here. Does the PR I sent to your fork make sense. Thanks!

@xmakro
Copy link
Contributor Author

xmakro commented Feb 25, 2025

Looks great, thanks! I merged your PR

@rjduffner
Copy link
Contributor

Nice thank you! @xmakro

@xrmx xrmx merged commit 39767ae into open-telemetry:main Feb 28, 2025
384 checks passed
@github-project-automation github-project-automation bot moved this from Ready for review to Done in @xrmx's Python PR digest Feb 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[gRPC Exporter] gRPC channel is not properly closed
4 participants