Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📖 Docs PR preview links
|
flippedcoder
left a comment
flippedcoder
left a comment
There was a problem hiding this comment.
Just a couple of small things to consider!
| layers that handle different concerns: | ||
|
|
||
| <CaptionedImage | ||
| src="/diagrams/data-converter-flow-with-external-storage.svg" |
There was a problem hiding this comment.
The arrows in and out of the container looked a little weird to me in dark mode. It could be fine, just worth another look.
There was a problem hiding this comment.
switched to themed images. i think maybe after this PR i will just update our captioned images component to have a dark/light theme
| @@ -0,0 +1,315 @@ | |||
| --- | |||
| id: large-payload-storage | |||
There was a problem hiding this comment.
Could the id be changed to external-storage to match the page title? Maybe external-storage-aws if the other cloud platforms are coming later.
docs/develop/go/best-practices/data-handling/large-payload-storage.mdx
Outdated
Show resolved
Hide resolved
…mentation into large-payload-go
| ```go | ||
| import ( | ||
| "github.com/aws/aws-sdk-go-v2/config" | ||
| awss3 "github.com/aws/aws-sdk-go-v2/service/s3" |
There was a problem hiding this comment.
Any particular reason why you aliased the import to awss3 instead of just using the default and calling s3.NewFromConfig?
| <!--SNIPEND--> | ||
|
|
||
| By default, payloads larger than 256 KiB are offloaded to external storage. You can adjust this with the | ||
| `PayloadSizeThreshold` option, even setting it to 0 to externalize all payloads regardless of size. Refer to |
There was a problem hiding this comment.
| `PayloadSizeThreshold` option, even setting it to 0 to externalize all payloads regardless of size. Refer to | |
| `PayloadSizeThreshold` option, even setting it to 1 to externalize all payloads regardless of size. Refer to |
The minimum value for externalizing all payloads in the Go SDK is 1. That is because the construction of the ExternalStorage struct already has this field set to 0. We interpret 0 to mean the default value.
|
|
||
| - `Name()` returns a unique string that identifies the driver instance. The SDK stores this name in the claim check | ||
| reference so it can route retrieval requests to the correct driver. Changing the name after payloads have been stored | ||
| breaks retrieval. For example, two S3 drivers could be named `"s3-us-east"` and `"s3-eu-west"`. |
There was a problem hiding this comment.
Not sure that drivers would actually be named after the region that they might be targeting. Maybe they'd be named for why they are stored differently.
| reference so it can route retrieval requests to the correct driver. Changing the name after payloads have been stored | ||
| breaks retrieval. For example, two S3 drivers could be named `"s3-us-east"` and `"s3-eu-west"`. | ||
| - `Type()` returns a string that identifies the driver implementation. Unlike `Name()`, this must be the same across all | ||
| instances of the same driver type regardless of configuration. Both S3 drivers in the example above would return |
There was a problem hiding this comment.
Both S3 drivers in the example above would return
"aws.s3driver"as their type.
Only the example at the top of the page would return this. The sample driver you wrote just above this would return local-disk.
There was a problem hiding this comment.
was referring the example in text s3-us-east etc, but will clarify here
| storeDir string | ||
| } | ||
|
|
||
| func NewLocalDiskStorageDriver(storeDir string) *LocalDiskStorageDriver { |
There was a problem hiding this comment.
Probably better to return StorageDriver instead of *LocalDiskStorageDriver.
| ## Configure payload size threshold | ||
|
|
||
| You can configure the payload size threshold that triggers external storage. By default, payloads larger than 256 KiB | ||
| are offloaded to external storage. You can adjust this with the `PayloadSizeThreshold` option, or set it to 0 to |
There was a problem hiding this comment.
Same comment about how 1 is the minimum for externalizing all payloads in Go.
| c, err := client.Dial(client.Options{ | ||
| ExternalStorage: converter.ExternalStorage{ | ||
| Drivers: []converter.StorageDriver{driver}, | ||
| PayloadSizeThreshold: 0, |
There was a problem hiding this comment.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
3 participants
What does this PR do?
Notes to reviewers
┆Attachments: EDU-6196 docs: add go docs for payload storage