You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "lidavidm (via GitHub)" <gi...@apache.org> on 2024/02/29 22:18:49 UTC
[PR] feat(go/adbc/driver/flightsql): reflect gRPC status in vendor code [arrow-adbc]
lidavidm opened a new pull request, #1577:
URL: https://github.com/apache/arrow-adbc/pull/1577
Does not work in C/C++/Python due to #1576.
Fixes #1574.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(go/adbc/driver/flightsql): reflect gRPC status in vendor code [arrow-adbc]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #1577:
URL: https://github.com/apache/arrow-adbc/pull/1577#discussion_r1509368227
##########
go/adbc/driver/flightsql/utils.go:
##########
@@ -92,19 +93,37 @@ func adbcFromFlightStatusWithDetails(err error, header, trailer metadata.MD, con
// XXX: must check both headers and trailers because some implementations
// (like gRPC-Java) will consolidate trailers into headers for failed RPCs
for key, values := range header {
- switch key {
- case "content-type", "grpc-status-details-bin":
+ switch {
+ case key == "content-type":
+ // Not useful info
continue
+ case key == "grpc-status-details-bin":
+ // gRPC library parses this above via grpcStatus.Proto()
+ continue
+ case strings.HasSuffix(key, "-bin"):
+ for _, value := range values {
+ // that's right, gRPC stuffs binary data into a "string"
+ details = append(details, &adbc.BinaryErrorDetail{Name: key, Detail: []byte(value)})
+ }
default:
for _, value := range values {
details = append(details, &adbc.TextErrorDetail{Name: key, Detail: value})
}
}
Review Comment:
Also, we already call Convert above...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(go/adbc/driver/flightsql): reflect gRPC status in vendor code [arrow-adbc]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #1577:
URL: https://github.com/apache/arrow-adbc/pull/1577#discussion_r1509367619
##########
go/adbc/driver/flightsql/utils.go:
##########
@@ -92,19 +93,37 @@ func adbcFromFlightStatusWithDetails(err error, header, trailer metadata.MD, con
// XXX: must check both headers and trailers because some implementations
// (like gRPC-Java) will consolidate trailers into headers for failed RPCs
for key, values := range header {
- switch key {
- case "content-type", "grpc-status-details-bin":
+ switch {
+ case key == "content-type":
+ // Not useful info
continue
+ case key == "grpc-status-details-bin":
+ // gRPC library parses this above via grpcStatus.Proto()
+ continue
+ case strings.HasSuffix(key, "-bin"):
+ for _, value := range values {
+ // that's right, gRPC stuffs binary data into a "string"
+ details = append(details, &adbc.BinaryErrorDetail{Name: key, Detail: []byte(value)})
+ }
default:
for _, value := range values {
details = append(details, &adbc.TextErrorDetail{Name: key, Detail: value})
}
}
Review Comment:
This is different. This is all the headers/trailers so we can reflect anything the server might send, not just the undocumented and partially-broken grpc-status-details-bin header that gRPC's Status covers.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(go/adbc/driver/flightsql): reflect gRPC status in vendor code [arrow-adbc]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #1577:
URL: https://github.com/apache/arrow-adbc/pull/1577#discussion_r1509358485
##########
go/adbc/driver/flightsql/utils.go:
##########
@@ -92,19 +93,37 @@ func adbcFromFlightStatusWithDetails(err error, header, trailer metadata.MD, con
// XXX: must check both headers and trailers because some implementations
// (like gRPC-Java) will consolidate trailers into headers for failed RPCs
for key, values := range header {
- switch key {
- case "content-type", "grpc-status-details-bin":
+ switch {
+ case key == "content-type":
+ // Not useful info
continue
+ case key == "grpc-status-details-bin":
+ // gRPC library parses this above via grpcStatus.Proto()
+ continue
+ case strings.HasSuffix(key, "-bin"):
+ for _, value := range values {
+ // that's right, gRPC stuffs binary data into a "string"
+ details = append(details, &adbc.BinaryErrorDetail{Name: key, Detail: []byte(value)})
+ }
default:
for _, value := range values {
details = append(details, &adbc.TextErrorDetail{Name: key, Detail: value})
}
}
Review Comment:
Could we leverage https://pkg.go.dev/google.golang.org/grpc@v1.62.0/status#Convert instead to get the message, error and details from the `err` instead of having to dig into the headers/trailers? Or is it not the same information?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(go/adbc/driver/flightsql): reflect gRPC status in vendor code [arrow-adbc]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm merged PR #1577:
URL: https://github.com/apache/arrow-adbc/pull/1577
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org