Add support for callback header #8
Conversation
bergundy
force-pushed
the
callback-header
branch
from
08ac5f6
to
726154c
Compare
nexus/api.go
Outdated
| // Nexus headers can only have single values, ignore multiple values. | ||
| header[strings.ToLower(k[8:])] = v[0] | ||
| header[lowerK[8:]] = v[0] |
There was a problem hiding this comment.
nit: I think putting this in a stripPrefix helper and then using len(prefix) here would be cleaner.
nexus/api.go
Outdated
| for k, v := range httpHeader { | ||
| lowerK := strings.ToLower(k) | ||
| if strings.HasPrefix(lowerK, "nexus-callback-") { | ||
| // Nexus headers can only have single values, ignore multiple values. |
There was a problem hiding this comment.
Just want to make sure, is this in the spec? It seems like a limitation people should be aware of. Also, why do we need this limitation?
There was a problem hiding this comment.
For compatibility with gRPC, Kafka, and Temporal headers.
This is the lowest common denominator.
There was a problem hiding this comment.
I thought I put it in the spec. I'll update, thanks.
nexus/api.go
Outdated
| lowerK := strings.ToLower(k) | ||
| if strings.HasPrefix(lowerK, "nexus-callback-") { | ||
| // Nexus headers can only have single values, ignore multiple values. | ||
| header[lowerK[15:]] = v[0] |
There was a problem hiding this comment.
nit: I know headers should be case insensitive, but it seems weird that we're forcing them to be lowercase now. Could we just do header[k[15:]] = v[0] instead?
There was a problem hiding this comment.
Sure, but it really doesn't matter.
| @@ -259,6 +260,9 @@ type ExecuteOperationOptions struct { | |||
| // Even though Client.ExecuteOperation waits for operation completion, some applications may want to set this | |||
| // callback as a fallback mechanism. | |||
| CallbackURL string | |||
| // Optional header fields set by a client that are required to be attached to the callback request when an | |||
| // asynchronous operation completes. | |||
| CallbackHeader Header | |||
There was a problem hiding this comment.
If there can only be one value per key here, consider just using a map.
There was a problem hiding this comment.
Header has a case insensitive getter, it's convenient.
Exposed callback headers as defined in this recent change to the API:
nexus-rpc/api#4