Skip to content

Conversation

cmaglie
Copy link
Member

@cmaglie cmaglie commented Aug 23, 2023

Please check if the PR fulfills these requirements

See how to contribute

  • The PR has no duplicates (please search among the Pull Requests
    before creating one)
  • The PR follows
    our contributing guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • UPGRADING.md has been updated with a migration guide (for breaking changes)
  • configuration.schema.json updated if new parameters are added.

What kind of change does this PR introduce?

Adds a close feature to the gRPC Monitor call. This new feature allows the client to gracefully close the port monitor.

What is the current behavior?

To close a gRPC-opened monitor, the client must close the gRPC streaming call, BTW this creates a race condition: the client is not aware of when the connection to the port is actually closed by the daemon afterward.
See #2271 for details.

What is the new behavior?

The close message should allow to gracefully close the monitor.

Does this PR introduce a breaking change, and is titled accordingly?

Yes, the gRPC Monitor call has been refactored to use a better description of the gRPC message based on the oneof descriptor.

Other information

Fix #2271

@cmaglie cmaglie added type: enhancement Proposed improvement topic: gRPC Related to the gRPC interface labels Aug 23, 2023
@cmaglie cmaglie self-assigned this Aug 23, 2023
@codecov
Copy link

codecov bot commented Aug 23, 2023

Codecov Report

Attention: 7 lines in your changes are missing coverage. Please review.

Comparison is base (07cf265) 68.71% compared to head (094e3a5) 68.85%.

Files Patch % Lines
commands/daemon/daemon.go 69.56% 4 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2276      +/-   ##
==========================================
+ Coverage   68.71%   68.85%   +0.13%     
==========================================
  Files         204      204              
  Lines       20446    20465      +19     
==========================================
+ Hits        14050    14091      +41     
+ Misses       5251     5220      -31     
- Partials     1145     1154       +9     
Flag Coverage Δ
unit 68.85% <73.07%> (+0.13%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cmaglie cmaglie force-pushed the monitor_close_grpc branch from 9534e2c to 094e3a5 Compare January 2, 2024 10:27
@cmaglie cmaglie merged commit 0dbd871 into arduino:master Jan 2, 2024
@cmaglie cmaglie deleted the monitor_close_grpc branch January 2, 2024 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: gRPC Related to the gRPC interface type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide a gRPC (messaging) way of terminating the monitor connection
2 participants