feat: add external storage support to codec server#4489
feat: add external storage support to codec server#4489lennessyy wants to merge 2 commits intocodec-server-refreshfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📖 Docs PR preview links
|
jmaeagle99
left a comment
jmaeagle99
left a comment
There was a problem hiding this comment.
Overall, this looks great to me. I added a few comments that talk about more nuanced distinctions.
| to handle downloading and decoding in the correct order for you to be able to view the Workflow data in the UI or CLI. | ||
|
|
||
| To support External Storage, configure your Codec Server with your storage drivers, your Payload Codec, and any | ||
| proxy-level codecs in a single `PayloadHTTPHandler` configuration. The handler applies them in the correct order across |
There was a problem hiding this comment.
Users may not find documentation that exactly has "PayloadHTTPHandler". We probably should instruct them that they can create one of these using NewPayloadHTTPHandler and it takes PayloadHTTPHandlerOptions that has fields for setting these three.
| to handle downloading and decoding in the correct order for you to be able to view the Workflow data in the UI or CLI. | ||
|
|
||
| To support External Storage, configure your Codec Server with your storage drivers, your Payload Codec, and any | ||
| proxy-level codecs in a single `PayloadHTTPHandler` configuration. The handler applies them in the correct order across |
There was a problem hiding this comment.
Probably not in this paragraph, but we should probably warn customers to not use the payload HTTP handler as a target of a remote data converter / codec. The reason being that this handler performs the entire encode-store-encode and decode-retrieve-decode pipeline instead of just encode or decode. The handler does have the same shape of the request and response structure that the remote data converter / codec expects, but is not intended to be used as such a target.
If users want to use remote data converters / codecs, they should set up those as they normally would using NewRemoteDataConverter in their worker and NewPayloadCodecHTTPHandler in their codec server. The should separately set up the NewPayloadHTTPHandler in the codec server for use by Web UI and CLI if external storage is involved and provide it the same codecs that NewPayloadCodecHTTPHandler is configured with.
| 3. The CLI sends the reference token to the Temporal Service, which stores it in the Event History. | ||
| 4. Later, a user views the Workflow in the Web UI. The Web UI retrieves the Event History from the Temporal Service and | ||
| sends the payloads to the Codec Server's `/decode` endpoint with the `?preserveStorageRefs=true` query parameter. | ||
| 5. The Codec Server decodes any payloads that are below the storage threshold through the Payload Codec, but returns |
There was a problem hiding this comment.
The codec server doesn't consider the threshold on the /decode path. What it's doing is using the proxy-level codecs for decoding first (in this example, there are none), then checks if the query parameter is true and bypasses the rest of the process for external storage references. If query parameter is false/missing or the payload is not an external storage reference, it will decode them with the worker-level codecs.
2 participants
What does this PR do?
Notes to reviewers
┆Attachments: EDU-6271 feat: add external storage support to codec server