Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

ARROW-12746: [Go][Flight] append instead of overwriting outgoing metadata #10297

Closed
wants to merge 1 commit into from

Conversation

zeroshade
Copy link
Member

No description provided.

@github-actions
Copy link

@zeroshade
Copy link
Member Author

@emkornfield another small bug fix that I found during some work

@@ -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...)
Copy link
Contributor

Choose a reason for hiding this comment

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

its not clear to me why append and not overwrite is the right thing here?

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

kszucs pushed a commit to kszucs/arrow that referenced this pull request May 17, 2021
…data

Closes apache#10297 from zeroshade/flight-client-metadata

Authored-by: Matthew Topol <mtopol@factset.com>
Signed-off-by: Micah Kornfield <emkornfield@gmail.com>
michalursa pushed a commit to michalursa/arrow that referenced this pull request Jun 13, 2021
…data

Closes apache#10297 from zeroshade/flight-client-metadata

Authored-by: Matthew Topol <mtopol@factset.com>
Signed-off-by: Micah Kornfield <emkornfield@gmail.com>
@zeroshade zeroshade deleted the flight-client-metadata branch September 12, 2021 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants