Deprecate remaining Limits, make all buffers adaptive #186
Conversation
cee-dub
force-pushed
the
cee-dub/limitless-headers
branch
2 times, most recently
from
930b72a to
83d5baa
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR deprecates remaining buffer size limits and makes all buffers adaptive to ease interaction with the platform. The key changes include refactoring buffer management to automatically resize when encountering BufLen errors and deprecating fixed-size limit APIs.
- Introduced adaptive buffer handling that automatically resizes when buffer capacity is exceeded
- Removed fixed buffer size parameters from HTTP header and method APIs
- Deprecated limit-related APIs in favor of adaptive buffer sizing
Reviewed Changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/abi/fastly/adaptivebuf.go | New helper function for adaptive buffer management |
| internal/abi/fastly/types.go | Refactored Values iterator to use adaptive buffering |
| fsthttp/limits.go | Deprecated Limits struct and related methods |
| Multiple guest.go files | Replaced fixed-size buffers with adaptive buffer calls |
| Multiple other files | Updated method signatures to remove buffer size parameters |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
cee-dub
force-pushed
the
cee-dub/limitless-headers
branch
from
83d5baa to
6685e83
Compare
| log_debug "Checking port with /dev/tcp/${host}/${port}" | ||
| fi | ||
|
|
||
| (true &>/dev/null <>/dev/tcp/${host}/${port}) |
There was a problem hiding this comment.
Do we want this to run on macOS? This is only going to work on Linux.
There was a problem hiding this comment.
It actually works fine on my Mac AFAICT. This /dev is a bash built-in.
There was a problem hiding this comment.
ah! ok, neat! did not know about that.
There was a problem hiding this comment.
I just learned about it, too. Will add a comment to explain.
| // Deprecated: the limit is not enforced, buffer sizing is adaptive. | ||
| func (limits *Limits) MaxHeaderNameLen() int { | ||
| return limits.maxHeaderNameLen | ||
| return math.MaxInt |
There was a problem hiding this comment.
I think we should continue to return the same default values as before -- this is what the Rust SDK did when these were marked deprecated. If not that, then we should probably make this a more realistic value. See https://docs.fastly.com/products/network-services-resource-limits#request-and-response-limits for the actual platform limits.
Eases interaction with the platform, which imposes limits elsewhere.