You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/05/11 22:28:42 UTC
[GitHub] [arrow] zeroshade opened a new pull request #10297: ARROW-12746: [Go][Flight] append instead of overwriting outgoing metadata
zeroshade opened a new pull request #10297:
URL: https://github.com/apache/arrow/pull/10297
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] emkornfield commented on a change in pull request #10297: ARROW-12746: [Go][Flight] append instead of overwriting outgoing metadata
Posted by GitBox <gi...@apache.org>.
emkornfield commented on a change in pull request #10297:
URL: https://github.com/apache/arrow/pull/10297#discussion_r630584409
##########
File path: go/arrow/flight/client_auth.go
##########
@@ -65,7 +65,7 @@ func createClientAuthUnaryInterceptor(auth ClientAuthHandler) grpc.UnaryClientIn
return status.Errorf(codes.Unauthenticated, "error retrieving token: %s", err)
}
- return invoker(metadata.NewOutgoingContext(ctx, metadata.Pairs(grpcAuthHeader, tok)), method, req, reply, cc, opts...)
Review comment:
its not clear to me why append and not overwrite is the right thing here?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] zeroshade commented on pull request #10297: ARROW-12746: [Go][Flight] append instead of overwriting outgoing metadata
Posted by GitBox <gi...@apache.org>.
zeroshade commented on pull request #10297:
URL: https://github.com/apache/arrow/pull/10297#issuecomment-839237015
@emkornfield another small bug fix that I found during some work
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] emkornfield closed pull request #10297: ARROW-12746: [Go][Flight] append instead of overwriting outgoing metadata
Posted by GitBox <gi...@apache.org>.
emkornfield closed pull request #10297:
URL: https://github.com/apache/arrow/pull/10297
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] zeroshade commented on a change in pull request #10297: ARROW-12746: [Go][Flight] append instead of overwriting outgoing metadata
Posted by GitBox <gi...@apache.org>.
zeroshade commented on a change in pull request #10297:
URL: https://github.com/apache/arrow/pull/10297#discussion_r630587448
##########
File path: go/arrow/flight/client_auth.go
##########
@@ -65,7 +65,7 @@ func createClientAuthUnaryInterceptor(auth ClientAuthHandler) grpc.UnaryClientIn
return status.Errorf(codes.Unauthenticated, "error retrieving token: %s", err)
}
- return invoker(metadata.NewOutgoingContext(ctx, metadata.Pairs(grpcAuthHeader, tok)), method, req, reply, cc, opts...)
Review comment:
the `ctx` variable passed in is the "context" that was created by the caller and passed. The user could add metadata (ie: headers) to the context by calling `NewOutgoingContext`. The problem is that by calling `NewOutgoingContext` here, we're dropping any metadata they had stuck in the context and replacing it with the auth metadata. So they're gonna lose any extra headers/metadata a caller intended on passing with the request.
By calling `AppendToOutgoingContext` if there is no metadata it'll create some, but if there already is metadata in the context that was placed there by the caller, we're just gonna add the authentication header to the metadata, preserving the caller's metadata instead of dropping it completely.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #10297: ARROW-12746: [Go][Flight] append instead of overwriting outgoing metadata
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #10297:
URL: https://github.com/apache/arrow/pull/10297#issuecomment-839236829
https://issues.apache.org/jira/browse/ARROW-12746
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org