❕ Gradle check result for 7c676dc: UNSTABLE

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

Codecov Report

❌ Patch coverage is 90.64039% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.00%. Comparing base (75fd8a6) to head (b7e6d83).
⚠️ Report is 2 commits behind head on main.

@@             Coverage Diff              @@
##               main   #19451      +/-   ##
============================================
+ Coverage     72.96%   73.00%   +0.03%     
- Complexity    70178    70281     +103     
============================================
  Files          5695     5700       +5     
  Lines        321939   322140     +201     
  Branches      46580    46614      +34     
============================================
+ Hits         234917   235173     +256     
+ Misses        68096    68045      -51     
+ Partials      18926    18922       -4     

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

✅ Gradle check result for 6c241a8: SUCCESS

Thanks @cwperks for the reviews.

Serialization errors can be more difficult to catch because they arise in mixed clusters when nodes of different versions are trying to communicate with one another. If 2 nodes are not speaking the same language (i.e. serializing/deserializing what the other expects), then they throw exceptions.

One thing to clarify - grpc transport is only applicable to client-server communication not node-to-node, so we shuold not be seeing failures in node to node comms. Though depending on which node the client hits, it could have different serialization results which is not ideal. I think it is easy to catch gRPC breaking changes though - as a breaking protobuf change is quite evident (e.g. fields are renamed or renumbered or removed).

I think what is the callenging is actually not to ensure grpc version x+1 does not introduce breaking changes from grpc version x; but rather, how to keep rest and grpc in sync. For this, for now we could consider something like this to handle it:

• case 1: removed or changed fields
  ideally, unit tests should exist for each rest field that map to a corresponding grpc field -> so if an existing rest field is changed the unit test fails and reminds the author to make a change to the grpc side too
  however today, a comprehensive set of unit tests like this does not exist for every single proto converter (some do). It's a known-ish issue, tracked in [Feature Request] GRPC Parity #18311
• case 2: added/new fields
  if a new field is added on rest side, it should also be added to the API specification. since the spec is used to convert to the protobufs, ideally a human reviewing the auto-generated protobufs should catch a new field was added and take it as an action item to add it to the server implementation too. This process can be somewhat automated/have alerts for it as well in the future

✅ Gradle check result for 33fa9b0: SUCCESS

❌ Gradle check result for 7aaf591: null

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

❌ Gradle check result for 8f030a6: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

❌ Gradle check result for 826dd69: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

❌ Gradle check result for 5832bc1: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

❕ Gradle check result for b7e6d83: UNSTABLE

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.