Skip to content

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

Closed
zeroshade wants to merge 1 commit intoapache:masterfrom
zeroshade:flight-client-metadata
Closed

ARROW-12746: [Go][Flight] append instead of overwriting outgoing metadata#10297
zeroshade wants to merge 1 commit intoapache:masterfrom
zeroshade:flight-client-metadata

Conversation

@zeroshade
Copy link
Copy Markdown
Member

No description provided.

@github-actions
Copy link
Copy Markdown

@zeroshade
Copy link
Copy Markdown
Member Author

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

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
Copy Markdown
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
Copy Markdown
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 <[email protected]>
Signed-off-by: Micah Kornfield <[email protected]>
@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.

2 participants