Skip to content

Commit b18760f

Browse files
mirkoCrobumirkoCrobu
andauthored
feat: enhance create app, fix mandatory model in upsert brick (#485)
Co-authored-by: mirkoCrobu <mirkocrobu@NB-0531.localdomain>
1 parent 0369ef5 commit b18760f

File tree

9 files changed

+72
-136
lines changed

9 files changed

+72
-136
lines changed

cmd/arduino-app-cli/app/new.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ func newCreateCmd() *cobra.Command {
2727
RunE: func(cmd *cobra.Command, args []string) error {
2828
cobra.MinimumNArgs(1)
2929
name := args[0]
30-
return createHandler(cmd.Context(), name, icon, bricks, noPyton, noSketch, fromApp)
30+
return createHandler(cmd.Context(), name, icon, noPyton, noSketch, fromApp)
3131
},
3232
}
3333

@@ -41,7 +41,7 @@ func newCreateCmd() *cobra.Command {
4141
return cmd
4242
}
4343

44-
func createHandler(ctx context.Context, name string, icon string, bricks []string, noPython, noSketch bool, fromApp string) error {
44+
func createHandler(ctx context.Context, name string, icon string, noPython, noSketch bool, fromApp string) error {
4545
if fromApp != "" {
4646
wd, err := paths.Getwd()
4747
if err != nil {
@@ -78,7 +78,6 @@ func createHandler(ctx context.Context, name string, icon string, bricks []strin
7878
resp, err := orchestrator.CreateApp(ctx, orchestrator.CreateAppRequest{
7979
Name: name,
8080
Icon: icon,
81-
Bricks: bricks,
8281
SkipPython: noPython,
8382
SkipSketch: noSketch,
8483
})

internal/api/docs/openapi.yaml

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1089,14 +1089,6 @@ components:
10891089
type: object
10901090
CreateAppRequest:
10911091
properties:
1092-
bricks:
1093-
description: application bricks
1094-
example:
1095-
- core-auth
1096-
- data-storage
1097-
items:
1098-
type: string
1099-
type: array
11001092
icon:
11011093
description: application icon
11021094
type: string

internal/api/handlers/app_create.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,8 @@ import (
1515
)
1616

1717
type CreateAppRequest struct {
18-
Name string `json:"name" description:"application name" example:"My Awesome App" required:"true"`
19-
Icon string `json:"icon" description:"application icon" `
20-
Bricks []string `json:"bricks,omitempty" description:"application bricks" example:"[\"core-auth\", \"data-storage\"]"`
18+
Name string `json:"name" description:"application name" example:"My Awesome App" required:"true"`
19+
Icon string `json:"icon" description:"application icon" `
2120
}
2221

2322
func HandleAppCreate(dockerClient *dockerClient.Client) http.HandlerFunc {
@@ -49,7 +48,6 @@ func HandleAppCreate(dockerClient *dockerClient.Client) http.HandlerFunc {
4948
orchestrator.CreateAppRequest{
5049
Name: req.Name,
5150
Icon: req.Icon,
52-
Bricks: req.Bricks,
5351
SkipPython: skipPython,
5452
SkipSketch: skipSketch,
5553
},

internal/e2e/client/client.gen.go

Lines changed: 0 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

internal/e2e/daemon/app_test.go

Lines changed: 32 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,9 @@ import (
1818
func TestCreateApp(t *testing.T) {
1919
httpClient := GetHttpclient(t)
2020

21-
bricks := []string{"brick_1"}
2221
defaultRequestBody := client.CreateAppRequest{
23-
Icon: f.Ptr("🌎"),
24-
Name: "HelloWorld",
25-
Bricks: &bricks,
22+
Icon: f.Ptr("🌎"),
23+
Name: "HelloWorld",
2624
}
2725

2826
testCases := []struct {
@@ -58,9 +56,8 @@ func TestCreateApp(t *testing.T) {
5856
SkipSketch: f.Ptr(false),
5957
},
6058
body: client.CreateAppRequest{
61-
Icon: f.Ptr("🌎"),
62-
Name: "HelloWorld_2",
63-
Bricks: &bricks,
59+
Icon: f.Ptr("🌎"),
60+
Name: "HelloWorld_2",
6461
},
6562
expectedStatusCode: http.StatusCreated,
6663
},
@@ -71,9 +68,8 @@ func TestCreateApp(t *testing.T) {
7168
SkipSketch: f.Ptr(true),
7269
},
7370
body: client.CreateAppRequest{
74-
Icon: f.Ptr("🌎"),
75-
Name: "HelloWorld_3",
76-
Bricks: &bricks,
71+
Icon: f.Ptr("🌎"),
72+
Name: "HelloWorld_3",
7773
},
7874
expectedStatusCode: http.StatusCreated,
7975
},
@@ -560,15 +556,24 @@ func TestAppDetails(t *testing.T) {
560556
t.Context(),
561557
&client.CreateAppParams{SkipSketch: f.Ptr(true)},
562558
client.CreateAppRequest{
563-
Icon: f.Ptr("💻"),
564-
Name: appName,
565-
Bricks: f.Ptr([]string{ImageClassifactionBrickID}),
559+
Icon: f.Ptr("💻"),
560+
Name: appName,
566561
},
567562
)
568563
require.NoError(t, err)
569564
require.Equal(t, http.StatusCreated, createResp.StatusCode())
570565
require.NotNil(t, createResp.JSON201)
571566

567+
resp, err := httpClient.UpsertAppBrickInstanceWithResponse(
568+
t.Context(),
569+
*createResp.JSON201.Id,
570+
ImageClassifactionBrickID,
571+
client.BrickCreateUpdateRequest{Model: f.Ptr("mobilenet-image-classification")},
572+
func(ctx context.Context, req *http.Request) error { return nil },
573+
)
574+
require.NoError(t, err)
575+
require.Equal(t, http.StatusOK, resp.StatusCode())
576+
572577
t.Run("DetailsOfApp", func(t *testing.T) {
573578
appID := createResp.JSON201.Id
574579
detailsResp, err := httpClient.GetAppDetailsWithResponse(t.Context(), *appID)
@@ -602,16 +607,25 @@ func TestAppPorts(t *testing.T) {
602607
t.Context(),
603608
&client.CreateAppParams{SkipSketch: f.Ptr(true)},
604609
client.CreateAppRequest{
605-
Icon: f.Ptr("💻"),
606-
Name: "test-app",
607-
Bricks: &[]string{StreamLitUi},
610+
Icon: f.Ptr("💻"),
611+
Name: "test-app",
608612
},
609613
func(ctx context.Context, req *http.Request) error { return nil },
610614
)
611615
require.NoError(t, err)
612616
require.Equal(t, http.StatusCreated, createResp.StatusCode())
613617
require.NotNil(t, createResp.JSON201)
614618

619+
respBrick, err := httpClient.UpsertAppBrickInstanceWithResponse(
620+
t.Context(),
621+
*createResp.JSON201.Id,
622+
StreamLitUi,
623+
client.BrickCreateUpdateRequest{},
624+
func(ctx context.Context, req *http.Request) error { return nil },
625+
)
626+
require.NoError(t, err)
627+
require.Equal(t, http.StatusOK, respBrick.StatusCode())
628+
615629
resp, err := httpClient.GetAppPorts(
616630
t.Context(),
617631
*createResp.JSON201.Id,
@@ -638,9 +652,8 @@ func TestAppPorts(t *testing.T) {
638652
t.Context(),
639653
&client.CreateAppParams{SkipSketch: f.Ptr(true)},
640654
client.CreateAppRequest{
641-
Icon: f.Ptr("💻"),
642-
Name: "test-app-2",
643-
Bricks: &[]string{ImageClassifactionBrickID},
655+
Icon: f.Ptr("💻"),
656+
Name: "test-app-2",
644657
},
645658
func(ctx context.Context, req *http.Request) error { return nil },
646659
)

internal/e2e/daemon/instance_bricks_test.go

Lines changed: 25 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -16,25 +16,37 @@ import (
1616
"github.com/bcmi-labs/orchestrator/internal/e2e/client"
1717
)
1818

19-
func TestGetAppBrickInstances(t *testing.T) {
20-
19+
func setupTestApp(t *testing.T) (*client.CreateAppResp, *client.ClientWithResponses) {
2120
httpClient := GetHttpclient(t)
22-
var actualBody models.ErrorResponse
23-
2421
createResp, err := httpClient.CreateAppWithResponse(
2522
t.Context(),
2623
&client.CreateAppParams{SkipSketch: f.Ptr(true)},
2724
client.CreateAppRequest{
28-
Icon: f.Ptr("💻"),
29-
Name: "test-app",
30-
Bricks: &[]string{ImageClassifactionBrickID},
25+
Icon: f.Ptr("💻"),
26+
Name: "test-app",
3127
},
3228
func(ctx context.Context, req *http.Request) error { return nil },
3329
)
3430
require.NoError(t, err)
3531
require.Equal(t, http.StatusCreated, createResp.StatusCode())
3632
require.NotNil(t, createResp.JSON201)
3733

34+
resp, err := httpClient.UpsertAppBrickInstanceWithResponse(
35+
t.Context(),
36+
*createResp.JSON201.Id,
37+
ImageClassifactionBrickID,
38+
client.BrickCreateUpdateRequest{Model: f.Ptr("mobilenet-image-classification")},
39+
func(ctx context.Context, req *http.Request) error { return nil },
40+
)
41+
require.NoError(t, err)
42+
require.Equal(t, http.StatusOK, resp.StatusCode())
43+
44+
return createResp, httpClient
45+
}
46+
47+
func TestGetAppBrickInstances(t *testing.T) {
48+
var actualBody models.ErrorResponse
49+
createResp, httpClient := setupTestApp(t)
3850
t.Run("GetAppBrickInstances_Success", func(t *testing.T) {
3951
brickInstances, err := httpClient.GetAppBrickInstancesWithResponse(t.Context(), *createResp.JSON201.Id, func(ctx context.Context, req *http.Request) error { return nil })
4052
require.NoError(t, err)
@@ -71,23 +83,8 @@ func TestGetAppBrickInstances(t *testing.T) {
7183

7284
func TestGetAppBrickInstanceById(t *testing.T) {
7385

74-
httpClient := GetHttpclient(t)
75-
7686
var actualBody models.ErrorResponse
77-
78-
createResp, err := httpClient.CreateAppWithResponse(
79-
t.Context(),
80-
&client.CreateAppParams{SkipSketch: f.Ptr(true)},
81-
client.CreateAppRequest{
82-
Icon: f.Ptr("💻"),
83-
Name: "test-app",
84-
Bricks: &[]string{ImageClassifactionBrickID},
85-
},
86-
func(ctx context.Context, req *http.Request) error { return nil },
87-
)
88-
require.NoError(t, err)
89-
require.Equal(t, http.StatusCreated, createResp.StatusCode())
90-
require.NotNil(t, createResp.JSON201)
87+
createResp, httpClient := setupTestApp(t)
9188

9289
t.Run("GetAppBrickInstanceByBrickID_Success", func(t *testing.T) {
9390
brickInstance, err := httpClient.GetAppBrickInstanceByBrickIDWithResponse(
@@ -138,34 +135,7 @@ func TestGetAppBrickInstanceById(t *testing.T) {
138135

139136
func TestUpsertAppBrickInstance(t *testing.T) {
140137
var actualResponseBody models.ErrorResponse
141-
httpClient := GetHttpclient(t)
142-
143-
createResp, err := httpClient.CreateAppWithResponse(
144-
t.Context(),
145-
&client.CreateAppParams{SkipSketch: f.Ptr(true)},
146-
client.CreateAppRequest{
147-
Icon: f.Ptr("💻"),
148-
Name: "test-app",
149-
},
150-
func(ctx context.Context, req *http.Request) error { return nil },
151-
)
152-
require.NoError(t, err)
153-
require.Equal(t, http.StatusCreated, createResp.StatusCode())
154-
require.NotNil(t, createResp.JSON201)
155-
156-
// Create the brick instance
157-
resp, err := httpClient.UpsertAppBrickInstanceWithResponse(
158-
t.Context(),
159-
*createResp.JSON201.Id,
160-
ImageClassifactionBrickID,
161-
client.BrickCreateUpdateRequest{
162-
Model: f.Ptr("person-classification"),
163-
Variables: &map[string]string{"CUSTOM_MODEL_PATH": "overidden"},
164-
},
165-
func(ctx context.Context, req *http.Request) error { return nil },
166-
)
167-
require.NoError(t, err)
168-
require.Equal(t, http.StatusOK, resp.StatusCode())
138+
createResp, httpClient := setupTestApp(t)
169139

170140
// Verify the brick instance was updated
171141
brickInstance, err := httpClient.GetAppBrickInstanceByBrickIDWithResponse(
@@ -176,8 +146,8 @@ func TestUpsertAppBrickInstance(t *testing.T) {
176146
require.NoError(t, err)
177147
require.NotEmpty(t, brickInstance.JSON200)
178148
require.Equal(t, ImageClassifactionBrickID, *brickInstance.JSON200.Id)
179-
require.Equal(t, "overidden", (*brickInstance.JSON200.Variables)["CUSTOM_MODEL_PATH"])
180-
require.Equal(t, "person-classification", *brickInstance.JSON200.Model)
149+
require.Equal(t, "/models/custom/ei/", (*brickInstance.JSON200.Variables)["CUSTOM_MODEL_PATH"])
150+
require.Equal(t, "mobilenet-image-classification", *brickInstance.JSON200.Model)
181151

182152
t.Run("OverrideBrickInstance", func(t *testing.T) {
183153
resp, err := httpClient.UpsertAppBrickInstanceWithResponse(
@@ -300,21 +270,8 @@ func TestUpsertAppBrickInstance(t *testing.T) {
300270

301271
func TestUpdateAppBrickInstance(t *testing.T) {
302272
var actualResponseBody models.ErrorResponse
303-
httpClient := GetHttpclient(t)
273+
createResp, httpClient := setupTestApp(t)
304274

305-
createResp, err := httpClient.CreateAppWithResponse(
306-
t.Context(),
307-
&client.CreateAppParams{SkipSketch: f.Ptr(true)},
308-
client.CreateAppRequest{
309-
Icon: f.Ptr("💻"),
310-
Name: "test-app",
311-
Bricks: &[]string{ImageClassifactionBrickID},
312-
},
313-
func(ctx context.Context, req *http.Request) error { return nil },
314-
)
315-
require.NoError(t, err)
316-
require.Equal(t, http.StatusCreated, createResp.StatusCode())
317-
require.NotNil(t, createResp.JSON201)
318275
t.Run("UpdateAppBrickInstance", func(t *testing.T) {
319276
resp, err := httpClient.UpdateAppBrickInstanceWithResponse(
320277
t.Context(),
@@ -480,21 +437,7 @@ func TestUpdateAppBrickInstance(t *testing.T) {
480437

481438
func TestDeleteAppBrickInstance(t *testing.T) {
482439

483-
httpClient := GetHttpclient(t)
484-
485-
createResp, err := httpClient.CreateAppWithResponse(
486-
t.Context(),
487-
&client.CreateAppParams{SkipSketch: f.Ptr(true)},
488-
client.CreateAppRequest{
489-
Icon: f.Ptr("💻"),
490-
Name: "test-app",
491-
Bricks: &[]string{ImageClassifactionBrickID},
492-
},
493-
func(ctx context.Context, req *http.Request) error { return nil },
494-
)
495-
require.NoError(t, err)
496-
require.Equal(t, http.StatusCreated, createResp.StatusCode())
497-
require.NotNil(t, createResp.JSON201)
440+
createResp, httpClient := setupTestApp(t)
498441

499442
t.Run("DeleteAppBrickInstance_NoExistingAppId_fail", func(t *testing.T) {
500443
var actualResponseBody models.ErrorResponse

internal/orchestrator/bricks.go

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -230,16 +230,15 @@ func BrickCreate(
230230

231231
brickInstance.ID = req.ID
232232

233-
if req.Model == nil {
234-
return fmt.Errorf("received empty model ")
235-
}
236-
models := modelsIndex.GetModelsByBrick(brickInstance.ID)
237-
idx := slices.IndexFunc(models, func(m modelsindex.AIModel) bool { return m.ID == *req.Model })
238-
if idx == -1 {
239-
return fmt.Errorf("model %s does not exsist", *req.Model)
240-
}
233+
if req.Model != nil {
234+
models := modelsIndex.GetModelsByBrick(brickInstance.ID)
235+
idx := slices.IndexFunc(models, func(m modelsindex.AIModel) bool { return m.ID == *req.Model })
236+
if idx == -1 {
237+
return fmt.Errorf("model %s does not exsist", *req.Model)
238+
}
241239

242-
brickInstance.Model = models[idx].ID
240+
brickInstance.Model = models[idx].ID
241+
}
243242
brickInstance.Variables = req.Variables
244243

245244
if brickIndex == -1 {

internal/orchestrator/orchestrator.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -484,7 +484,6 @@ func AppDetails(
484484
type CreateAppRequest struct {
485485
Name string
486486
Icon string
487-
Bricks []string
488487
SkipPython bool
489488
SkipSketch bool
490489
}
@@ -544,10 +543,7 @@ if __name__ == "__main__":
544543
Name: req.Name,
545544
Description: "",
546545
Ports: []int{},
547-
Bricks: f.Map(req.Bricks, func(v string) app.Brick {
548-
return app.Brick{ID: v}
549-
}),
550-
Icon: req.Icon, // TODO: not sure if icon will exists for bricks
546+
Icon: req.Icon, // TODO: not sure if icon will exists for bricks
551547
},
552548
)
553549
if err != nil {

internal/orchestrator/orchestrator_test.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,8 @@ func TestCreateApp(t *testing.T) {
1818
setTestOrchestratorConfig(t)
1919
t.Run("valid app", func(t *testing.T) {
2020
r, err := CreateApp(t.Context(), CreateAppRequest{
21-
Name: "example app",
22-
Icon: "😃",
23-
Bricks: []string{"arduino:object-detection"},
21+
Name: "example app",
22+
Icon: "😃",
2423
})
2524
require.NoError(t, err)
2625
require.Equal(t, f.Must(ParseID("user:example-app")), r.ID)

0 commit comments

Comments
 (0)