Skip to content

[breaking] Some gRPC API improvements #2472

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Dec 21, 2023

Conversation

cmaglie
Copy link
Member

@cmaglie cmaglie commented Dec 19, 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?

Add the oneof messsage { ... } to the protobuf definition of the gRPC responses that provide a streaming output. This will make the gRPC consistent as requested in #2450.

This change also fixed a weird edge case in the gRPC->result mapping in the feedback package.

What is the current behavior?

No changes

What is the new behavior?

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

Yes.

Other information

Fix #2450

@cmaglie cmaglie added type: enhancement Proposed improvement topic: gRPC Related to the gRPC interface labels Dec 19, 2023
@cmaglie cmaglie self-assigned this Dec 19, 2023
@cmaglie cmaglie force-pushed the grpc-api-improements branch from b58ca48 to 0305e5e Compare December 19, 2023 12:32
Copy link

codecov bot commented Dec 19, 2023

Codecov Report

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

Comparison is base (372656a) 67.61% compared to head (0305e5e) 67.54%.

Files Patch % Lines
commands/daemon/daemon.go 44.00% 28 Missing ⚠️
internal/cli/compile/compile.go 50.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2472      +/-   ##
==========================================
- Coverage   67.61%   67.54%   -0.08%     
==========================================
  Files         207      207              
  Lines       20710    20747      +37     
==========================================
+ Hits        14003    14013      +10     
- Misses       5563     5589      +26     
- Partials     1144     1145       +1     
Flag Coverage Δ
unit 67.54% <48.33%> (-0.08%) ⬇️

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.

Copy link
Contributor

@kittaakos kittaakos left a comment

Choose a reason for hiding this comment

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

It's looking great. Thank you!

@cmaglie cmaglie merged commit 6732ae0 into arduino:master Dec 21, 2023
@cmaglie cmaglie deleted the grpc-api-improements branch December 21, 2023 15:24
# for free to join this conversation on GitHub. Already have an account? # 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 consistent gRPC interface for client streaming requests with stdout/stderr
3 participants