Arguments validations (1/3): add arguments_schema field and preserve optional fields on program re-upload#2330
Arguments validations (1/3): add arguments_schema field and preserve optional fields on program re-upload#2330avilches wants to merge 6 commits into
arguments_schema field and preserve optional fields on program re-upload#2330Conversation
avilches
changed the title
arguments_schema field and preserve optional fields on program re-upload
70948d3 to
d04cc53
Compare
| migrations.AddField( | ||
| model_name="program", | ||
| name="arguments_schema", | ||
| field=models.TextField(blank=True, default="{}", null=True), |
|
Draft until deployment in production works |
|
The PR is safe to merge: it adds a nullable field, so it is rollback compatible (old code inserting a program row just leaves the column NULL, which the read path treats as "no schema"). It also changes the /upload endpoint behaviour for the better. Before the PR, re-uploading a function to update it required sending entrypoint or image every time, and any optional field you left out (runner, dependencies, env_vars) was reset to its default. Now only the title is mandatory on update; every other field is preserved from the database when omitted. So to update a schema you only need to send the title plus the arguments_schema field. |
avilches
changed the title
arguments_schema field and preserve optional fields on program re-uploadarguments_schema field and preserve optional fields on program re-upload
avilches
changed the title
arguments_schema field and preserve optional fields on program re-uploadarguments_schema field and preserve optional fields on program re-upload
|
Safe to merge to main since pentest finished |
ElePT
left a comment
ElePT
left a comment
There was a problem hiding this comment.
The PR mostly looks good, I just find the runner change a bit strange. In practice I think this affects very few users but given how the runner has proven to be complicated to use before, I want to make sure we don't add even more hidden behaviors (what's a hidden behavior I guess is another discussion we can have).
| if data.runner is not None: | ||
| instance.runner = data.runner |
There was a problem hiding this comment.
I'm not convinced about preserving the runner value on re-upload. If someone omits runner, silently keeping fleets when they expected ray breaks job execution with no obvious error. The current SDK always sends runner anyway (it defaults to "ray"), so the only callers affected by this change are old SDK versions or direct API users, and for those, I think that falling back to ray is safer than preserving whatever was stored.
There was a problem hiding this comment.
Should we send ray by default in case of re-upload? I think someone can reupload a fleet function without specifying and have errors.
3 participants
Summary
Add an
arguments_schemafield to programs so a function can declare a JSON Schema describing its valid arguments, and store it on upload. This is the storage layer for argument validation; the validation itself and the client SDK support land in follow-up PRs.This also makes re-uploads non-destructive: re-uploading a function with only some fields no longer resets
runner,dependencies, orenv_varsto their defaults, and theentrypoint/imagerequirement now applies only when creating a function, not on update.Details and comments
arguments_schemais a text field validated as JSON at the serializer level (schema semantics are checked later, when validating arguments against it).Noneas the sentinel in the upload input dataclass.0055_program_arguments_schema.py.