-
Notifications
You must be signed in to change notification settings - Fork 137
Add support for gRPC binary metadata values #1791
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
base: main
Are you sure you want to change the base?
Conversation
fa18a14
to
2235a4f
Compare
let value = obj.get_value(cx, key_handle)?; | ||
map.insert(key, T::try_from_js(cx, value)?); | ||
let js_value = obj.get_value(cx, key_handle)?; | ||
let value = T::try_from_js(cx, js_value).field(&key)?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improves the error message slightly by indicating which field on the object had the type issue
let Some(headers) = headers else { | ||
return (None, None); | ||
}; | ||
let mut ascii_headers = HashMap::default(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possible optimization is to create the hashmap with the same capacity as the initial headers map. Only downside I could see would be if the headers were mostly binary.
1b17bc4
to
def0d6c
Compare
def0d6c
to
b9ec854
Compare
What was changed
Added ability to send binary gRPC headers via
NativeConnection
.Also added a test to verify that binary headers were accepted by
Connection
.Why?
Make progress on temporalio/features#671
Checklist
Closes [Feature Request] Ensure gRPC binary metadata headers are supported #1778
How was this tested:
Added setting and sending of binary headers at both the client and call level for both native and the standard client connection.
Any docs updates needed?
I believe the type expansion from
string
tostring | Buffer
is the only user facing change and that change should appear in the API reference.Each commit is reviewable on its own.