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

[RSDK-9149] Default gRPC Server to 'Wait For Handlers' #406

Merged
merged 4 commits into from
Jan 21, 2025

Conversation

bashar-515
Copy link
Member

@bashar-515 bashar-515 commented Jan 21, 2025

#405 Previously introduced a test failure in RDK due to a leaked Goroutine. The fix - put up in the same PR - was to always use GracefulStop() when closing the gRPC server. This is causing other tests (see the output logs here) to fail :/ As an alternative, this PR introduces the option to have a web server gracefully (or not) stop a gRPC server. This PR goes with this one, which will actually use said option in the initial tests that failed to avoid the leaked Goroutines. Once this merges, we should make a release and create a PR in RDK to run our CI tests.

@viambot viambot added the safe to test Mark as safe to test label Jan 21, 2025
@viambot viambot added safe to test Mark as safe to test and removed safe to test Mark as safe to test labels Jan 21, 2025
@viambot viambot added safe to test Mark as safe to test and removed safe to test Mark as safe to test labels Jan 21, 2025
@bashar-515 bashar-515 requested review from cheukt and dgottlieb and removed request for dgottlieb and cheukt January 21, 2025 19:59
@viambot viambot added safe to test Mark as safe to test and removed safe to test Mark as safe to test labels Jan 21, 2025
@bashar-515 bashar-515 requested a review from cheukt January 21, 2025 20:17
@bashar-515 bashar-515 marked this pull request as ready for review January 21, 2025 20:22
Copy link
Member

@cheukt cheukt left a comment

Choose a reason for hiding this comment

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

can you update the pr title?

@bashar-515 bashar-515 changed the title [RSDK-9149] Introduce Option to Gracefully Stop gRPC Server [RSDK-9149] Default gRPC Server to 'Wait For Handlers' Jan 21, 2025
@bashar-515 bashar-515 merged commit 332545c into viamrobotics:main Jan 21, 2025
6 of 8 checks passed
@bashar-515 bashar-515 deleted the RSDK-9149-options branch January 21, 2025 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
safe to test Mark as safe to test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants