Add Nexus Callback Token to StartOperationOptions#61
Add Nexus Callback Token to StartOperationOptions#61chaptersix wants to merge 9 commits intonexus-rpc:mainfrom
Conversation
|
@chaptersix please add a test and also make sure to rehydrate the option from the header. You will want to delete that header key so it doesn't show up in |
@bergundy should the sdk require the user pass in the callbacktoken? |
chaptersix
changed the title
chaptersix
force-pushed
the
alex/token
branch
from
4eaed9b to
3ab7506
Compare
I don't think this is relevant now. |
It's still relevant even though we inlined the implementation in the Temporal codebase. |
chaptersix
changed the title
Co-authored-by: Roey Berman <roey.berman@gmail.com>
| // CallbackToken is a caller-generated token that is used to recreate context when processing the callback. | ||
| // CallbackToken must be provided if CallbackURL is not empty. | ||
| // Handlers must Include the CallbackToken when they invoke the callback. | ||
| CallbackToken []byte |
There was a problem hiding this comment.
Token is just a header and this is already present in the header, along with all my other callback headers right? Or is this one header named "Token" special?
There was a problem hiding this comment.
We are giving this one special treatment because we want to abstract headers away from users.
On the caller side, they will provide an object that will be embedded into a token and let the framework handle serialization.
Now that I'm thinking about it, we should add this already here.
There was a problem hiding this comment.
I don't think we should abstract/hide headers from general Nexus users. Users should be allowed to set whatever headers, or 0 headers. If they happen to want some kind of "key" or "token" or "id" or whatever header they can, but we shouldn't require it or have it be a special header or whatever. From a Nexus POV, I should be able to have any headers I want in the callback (within reason), and ask the server to send those.
If Temporal happens to have a concept of "callback token", that doesn't mean others must too.
|
Removed myself as a reviewer. Let get back to this when it becomes higher priority. |
3 participants
In support of this feature this PR added the CallbackToken to the StartOperationOptions.