-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sandbox vs webclient behavior for nested arguments #538
Comments
The following example fully reproduces the issue in #537 within the sandbox, with no need to use actual server/client instances. We use this python script import json
from typing import Optional
from pydantic import validate_call, BaseModel, Field
from fractal_tasks_core.dev.lib_args_schemas import create_schema_for_single_task
class ComplexArg(BaseModel):
sub_attr_1: int = 99
@validate_call
def task_function(
arg_A: ComplexArg = Field(default_factory=ComplexArg),
arg_B: int = 1,
):
"""
Short task description
Args:
arg_A: Description of arg_A.
arg_B: Description of arg_B.
"""
pass
schema = create_schema_for_single_task(
task_function=task_function,
executable=__file__,
package=None,
)
print(json.dumps(schema, indent=2)) and generate this schema {
"$defs": {
"ComplexArg": {
"properties": {
"sub_attr_1": {
"default": 99,
"title": "Sub Attr 1",
"type": "integer"
}
},
"title": "ComplexArg",
"type": "object",
"description": "Missing description for ComplexArg."
}
},
"additionalProperties": false,
"properties": {
"arg_A": {
"$ref": "#/$defs/ComplexArg",
"title": "Arg A",
"description": "Description of arg_A."
},
"arg_B": {
"default": 1,
"title": "Arg B",
"type": "integer",
"description": "Description of arg_B."
}
},
"type": "object",
"title": "TaskFunction"
} If we use this schema in the sandbox without setting any JSON data (case A), we do see the 99 default: However, if we include the same schema and also set the data to {
"arg_B": 1
} |
This was fixed in fractal-server 1.3.6, where we stopped pre-populating the defaults from the backend.
Right now, the behavior in the webclient when a task is added for the first time is the same as in the sandbox. |
With @zonia3000 we reviewed and now better understand the difference between the sandbox and the webclient (ref #537). The brief explanation is that the backend most often provides some initial arguments data, while the sandbox typically uses empty data. This does not seem related to any recent change in fractal-web, but it became evident due to recent fractal-tasks-core updates (notably as of fractal-tasks-core 1.1.0, 1.1.1 and 1.2.0 versions, where the cellpose-task call signature was changed).
Side note: this unexpected difference probably already entered previous discussions, e.g. in this comment.
Current status:
The fractal-web UI generation behaves differently depending on whether the arguments object is empty (case A) or has some values (case B). In case A, it's "easy" to display defaults for all properties (top-level or nested). This is more tricky in case B - to be reviewed elsewhere.
In the sandbox, one mostly never sets any values in the "Initial JSON data" field - so that we end up in case A. Note that by manyally setting some data you can make the sandbox behave as in case B.
In an actual webclientweb instance, there is most often some value that is set (see next point) - so that we are in case B.
When creating a task (either via task collection or via single-task creation),
fractal-server
checks whether there is a JSON schema associated with it. If so, it looks fordefault
values of top-level schema arguments, and sets them as part of the arguments object. For this reason, any task that has at least one top-level default value will always end up in case B.Even if the task has no top-level defaul values, and then starts being in case A, after patching some of its arguments it will readily move to case B (as soon as the JSON data includes at least one value).
Improvements
Short term, we should at least make the sandbox mimic what the backend does - by prepopulating the top-level defaults in data.
On the backend side, I think we could deprecate this pre-populating feature. It's TBD, but my first guess is that this is a legacy feature that made sense when we were mostly using CLI clients and didn't want to write down a lot of arguments.
More complex, but clearly relevant, we can review the way fractal-web displays defaults of nested arguments.
[continues with an example]
The text was updated successfully, but these errors were encountered: