file_mount validation limits for python client #660
Conversation
| # Measure size in bytes using UTF-8 encoding since payloads are JSON strings | ||
| size_bytes = len(content.encode("utf-8")) | ||
| if size_bytes > FILE_MOUNT_MAX_SIZE_BYTES: | ||
| raise ValueError( |
There was a problem hiding this comment.
Might be a good idea to catch all large files instead of just the first one encountered.
There was a problem hiding this comment.
Great feedback! Making this change now. Thank you
|
|
||
| if total_size_bytes > FILE_MOUNT_TOTAL_MAX_SIZE_BYTES: | ||
| raise ValueError( | ||
| f"total file_mounts size exceeds maximum of {FILE_MOUNT_TOTAL_MAX_SIZE_BYTES} bytes. Use object_mounts instead." |
There was a problem hiding this comment.
Will we expect users to use file_mounts in combination with object_mounts? If so, it might be helpful to display what their total size is or how much they've exceeded the MAX_SIZE by, so they can make an informed partition of file_mounts vs object_mounts.
There was a problem hiding this comment.
I think the API needs to be flexible enough to support arbitrary combinations of object_mounts and file_mounts but the client-side should make it easy to do the right thing automatically (presumably by favoring object_mounts).
I think this change is worth making, but I wanted to provide more context as to why.
src/runloop_api_client/_constants.py
Outdated
| MAX_RETRY_DELAY = 60.0 | ||
|
|
||
| # Maximum allowed size (in bytes) for individual entries in `file_mounts` when creating Blueprints | ||
| FILE_MOUNT_MAX_SIZE_BYTES = 512 * 1024 |
There was a problem hiding this comment.
Should we keep these limits static or adjust them based on Devbox resource allocation?
There was a problem hiding this comment.
This limit isn't a devbox limitation; it's a blueprint dockerfile length constraint. I think that each file_mount performs a text substitution inside the dockerfile, so the problem is that the dockerfile becomes too massive; not that the file_mounts exhaust resources for the devbox the dockerfile gets compiled on
There was a problem hiding this comment.
Might be a good idea to test that we don't reject file mounts when under or precisely at the limit
There was a problem hiding this comment.
These are just client-side validations. There's no point being hyper-precise about these limits since they're really enforced on the server side anyway -- there's nothing to stop a caller ignoring our client & using our API endpoints directly.
…f fast fail with exception
Adds a cap to individual and total file_mount size