-
Notifications
You must be signed in to change notification settings - Fork 1
feat(api): return variable configurations in /apps/:id/bricks and apps/:id/bricks/:id
#18
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not using a list of variables like this instead of the object with the name of the variable as key?
"variables":[
{
"name: "CUSTOM_MODEL_PATH",
"default_value": "/home/arduino/.arduino-bricks/ei-models",
"description": "path to the custom model directory",
"required": false
},
{
"name" "EI_CLASSIFICATION_MODEL",
"default_value": "/models/ootb/ei/mobilenet-v2-224px.eim",
"description": "path to the model file",
"required": false
}
],
|
@dido18 I kept the same structure as |
d43f0e5 to
1c1fc7e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Could you add an example of the new json respone in the PR description ?
Thanks
|
GET http://localhost:8080/v1/apps/ZXhhbXBsZXM6aW1hZ2UtY2xhc3NpZmljYXRpb24/bricks |
|
/apps/:id/bricks and apps/{appID}/bricks/:id
/apps/:id/bricks and apps/{appID}/bricks/:id/apps/:id/bricks and apps/:id/bricks/:id
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Motivation
To complete the new flow “RUN an app with mandatory configuration”, the mandatory bricks variables should be set before running the app.
Currently, the App Lab to retrieve a mandatory app from an app must do the following:
/v1/apps/{appID}/bricksto list all the instance bricks for the current app(it doesn’t return the bricks variables information)/v1/apps/{appID}/bricks/{brickID}to check brickInstance variables current value(the response does not contain information about the mandatory requirement of a variable, but just the value)./v1/bricks/{brickID}to check which variable is mandatory.This set of calls is tedious and inefficient.
Change description
Integrate the
/v1/apps/{appID}/bricks/{brickID}and/v1/apps/{appID}/bricks, adding for each brickInstance theconfig_variablesarray with the list of variables with their complete information.Additional Notes
Reviewer checklist
main.