Skip to content

Conversation

MathieuTricoire
Copy link
Contributor

Motivation

The upstream Google proto file error_details.proto has been updated (see: https://github.com/googleapis/googleapis/blob/2e1c38ddaa402303b9c1381f5f9ba5a9ced4a51c/google/rpc/error_details.proto):

  • QuotaFailure message:
    • Added fields: api_service, quota_metric, quota_id, quota_dimensions, quota_value and future_quota_value
  • BadRequest.FieldViolation message:
    • Added fields: reason and localized_message

Modifications

Update the google proto files and generates the stubs using the provided scripts dev/protos/fetch.sh and dev/protos/generate.sh.

Result

Library users can now access the newly added fields in error_details.proto.

@glbrntt glbrntt added the 🔨 semver/patch No public API change. label Jul 28, 2025
Copy link
Collaborator

@glbrntt glbrntt left a comment

Choose a reason for hiding this comment

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

Thanks @MathieuTricoire; unfortunately this isn't actually sufficient for you to access these fields as we provide wrappers around these types. See here. Is that something you feel like contributing?

@MathieuTricoire
Copy link
Contributor Author

@glbrntt Ah, got it — sorry, I rushed it a bit. I’ll update the wrappers as well! 👍

@MathieuTricoire
Copy link
Contributor Author

I've pushed the changes for the wrappers, I don't have much time today to have a deeper look and check if I'm missing something else or if my code blend in correctly, but feel free to take a first look and let me know what you think.

I've added new initializers for QuotaFailure.Violation and BadRequest.FieldViolation to avoid introducing a breaking change. Let me know if you'd prefer a different approach, like updating the default initializer with default values, going ahead with a breaking change or something else.

Copy link
Collaborator

@glbrntt glbrntt left a comment

Choose a reason for hiding this comment

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

Thanks @MathieuTricoire. For some uninteresting reasons new API needs to be annotated with appropriate availability, I've left these as suggestions on the PR to save you from having to find them all.

Beyond that this looks good aside from one small nit re: Int/Int64

@MathieuTricoire
Copy link
Contributor Author

@glbrntt Thanks! I've added the availability annotations and fixed the Int/Int64 issue (I should have seen this one...)
Happy to make more changes if you notice anything else, is the string representation ok?

Copy link
Collaborator

@glbrntt glbrntt left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM!

@glbrntt glbrntt added 🆕 semver/minor Adds new public API. and removed 🔨 semver/patch No public API change. labels Jul 29, 2025
Copy link
Collaborator

@glbrntt glbrntt left a comment

Choose a reason for hiding this comment

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

Ah, Swift 6.0 CI is unhappy with the extra ,

/grpc-swift-protobuf/Sources/GRPCProtobuf/Errors/ErrorDetails+Types.swift:282:7: error: unexpected ',' separator
280 |         subject: String,
281 |         description: String,
282 |       ) {
    |       `- error: unexpected ',' separator
283 |         self.subject = subject
284 |         self.violationDescription = description

/grpc-swift-protobuf/Sources/GRPCProtobuf/Errors/ErrorDetails+Types.swift:505:7: error: unexpected ',' separator
503 |         field: String,
504 |         description: String,
505 |       ) {
    |       `- error: unexpected ',' separator
506 |         self.field = field
507 |         self.violationDescription = description

@MathieuTricoire
Copy link
Contributor Author

Oops sorry I'll fix that

@glbrntt glbrntt merged commit 9ac928d into grpc:main Jul 29, 2025
31 checks passed
dongjoon-hyun added a commit to apache/spark-connect-swift that referenced this pull request Aug 26, 2025
### What changes were proposed in this pull request?

This PR aims to upgrade `grpc-swift-protobuf` to 2.1.1.

### Why are the changes needed?

To bring the latest improvements and bug fixes.
- https://github.com/grpc/grpc-swift-protobuf/releases/tag/2.1.1
- https://github.com/grpc/grpc-swift-protobuf/releases/tag/2.1.0
  - grpc/grpc-swift-protobuf#77
  - grpc/grpc-swift-protobuf#82
  - grpc/grpc-swift-protobuf#81

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Pass the CIs.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes #217 from dongjoon-hyun/SPARK-53370.

Authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🆕 semver/minor Adds new public API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants