From a29820ebe3bc1f6f82f9a485bb373fb4aee75482 Mon Sep 17 00:00:00 2001 From: JP Ungaretti Date: Tue, 1 Nov 2022 21:00:14 +0000 Subject: [PATCH 1/4] Use incremental rebuild by default --- pkg/cmd/codespace/common.go | 2 +- pkg/cmd/codespace/rebuild.go | 8 +++-- pkg/cmd/codespace/rebuild_test.go | 2 +- pkg/liveshare/session.go | 9 ++++-- pkg/liveshare/session_test.go | 52 ++++++++++++++++++++----------- 5 files changed, 48 insertions(+), 25 deletions(-) diff --git a/pkg/cmd/codespace/common.go b/pkg/cmd/codespace/common.go index ee25854dc..ba0e8a9f6 100644 --- a/pkg/cmd/codespace/common.go +++ b/pkg/cmd/codespace/common.go @@ -66,7 +66,7 @@ type liveshareSession interface { StartSharing(context.Context, string, int) (liveshare.ChannelID, error) StartSSHServer(context.Context) (int, string, error) StartSSHServerWithOptions(context.Context, liveshare.StartSSHServerOptions) (int, string, error) - RebuildContainer(context.Context) error + RebuildContainer(context.Context, bool) error } // Connects to a codespace using Live Share and returns that session diff --git a/pkg/cmd/codespace/rebuild.go b/pkg/cmd/codespace/rebuild.go index f2128d632..273627f07 100644 --- a/pkg/cmd/codespace/rebuild.go +++ b/pkg/cmd/codespace/rebuild.go @@ -10,22 +10,24 @@ import ( func newRebuildCmd(app *App) *cobra.Command { var codespace string + var fullRebuild bool rebuildCmd := &cobra.Command{ Use: "rebuild", Short: "Rebuild a codespace", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { - return app.Rebuild(cmd.Context(), codespace) + return app.Rebuild(cmd.Context(), codespace, fullRebuild) }, } rebuildCmd.Flags().StringVarP(&codespace, "codespace", "c", "", "Name of the codespace") + rebuildCmd.Flags().BoolVar(&fullRebuild, "full", false, "Perform a full rebuild of the codespace") return rebuildCmd } -func (a *App) Rebuild(ctx context.Context, codespaceName string) (err error) { +func (a *App) Rebuild(ctx context.Context, codespaceName string, full bool) (err error) { ctx, cancel := context.WithCancel(ctx) defer cancel() @@ -46,7 +48,7 @@ func (a *App) Rebuild(ctx context.Context, codespaceName string) (err error) { } defer safeClose(session, &err) - err = session.RebuildContainer(ctx) + err = session.RebuildContainer(ctx, full) if err != nil { return fmt.Errorf("rebuilding codespace via session: %w", err) } diff --git a/pkg/cmd/codespace/rebuild_test.go b/pkg/cmd/codespace/rebuild_test.go index fff40fe1b..ec8b112de 100644 --- a/pkg/cmd/codespace/rebuild_test.go +++ b/pkg/cmd/codespace/rebuild_test.go @@ -15,7 +15,7 @@ func TestAlreadyRebuildingCodespace(t *testing.T) { } app := testingRebuildApp(*rebuildingCodespace) - err := app.Rebuild(context.Background(), "rebuildingCodespace") + err := app.Rebuild(context.Background(), "rebuildingCodespace", false) if err != nil { t.Errorf("rebuilding a codespace that was already rebuilding: %v", err) } diff --git a/pkg/liveshare/session.go b/pkg/liveshare/session.go index 40bdf287b..39fd46e5f 100644 --- a/pkg/liveshare/session.go +++ b/pkg/liveshare/session.go @@ -123,9 +123,14 @@ func (s *Session) StartJupyterServer(ctx context.Context) (int, string, error) { return port, response.ServerUrl, nil } -func (s *Session) RebuildContainer(ctx context.Context) error { +func (s *Session) RebuildContainer(ctx context.Context, full bool) error { + rpcMethod := "IEnvironmentConfigurationService.incrementalRebuildContainer" + if full { + rpcMethod = "IEnvironmentConfigurationService.rebuildContainer" + } + var rebuildSuccess bool - err := s.rpc.do(ctx, "IEnvironmentConfigurationService.rebuildContainer", nil, &rebuildSuccess) + err := s.rpc.do(ctx, rpcMethod, nil, &rebuildSuccess) if err != nil { return fmt.Errorf("invoking rebuild RPC: %w", err) } diff --git a/pkg/liveshare/session_test.go b/pkg/liveshare/session_test.go index 06000c344..b9dc91e9c 100644 --- a/pkg/liveshare/session_test.go +++ b/pkg/liveshare/session_test.go @@ -400,26 +400,42 @@ func TestSessionHeartbeat(t *testing.T) { } func TestRebuild(t *testing.T) { - requestCount := 0 - getSharedServers := func(conn *jsonrpc2.Conn, req *jsonrpc2.Request) (interface{}, error) { - requestCount++ - return true, nil - } - testServer, session, err := makeMockSession( - livesharetest.WithService("IEnvironmentConfigurationService.rebuildContainer", getSharedServers), - ) - if err != nil { - t.Fatalf("creating mock session: %v", err) - } - defer testServer.Close() - - err = session.RebuildContainer(context.Background()) - if err != nil { - t.Fatalf("rebuilding codespace via mock session: %v", err) + tests := []struct { + fullRebuild bool + rpcService string + }{ + { + fullRebuild: false, + rpcService: "IEnvironmentConfigurationService.incrementalRebuildContainer", + }, + { + fullRebuild: true, + rpcService: "IEnvironmentConfigurationService.rebuildContainer", + }, } - if requestCount == 0 { - t.Fatalf("no requests were made") + for _, tt := range tests { + requestCount := 0 + getSharedServers := func(conn *jsonrpc2.Conn, req *jsonrpc2.Request) (interface{}, error) { + requestCount++ + return true, nil + } + testServer, session, err := makeMockSession( + livesharetest.WithService(tt.rpcService, getSharedServers), + ) + if err != nil { + t.Fatalf("creating mock session: %v", err) + } + defer testServer.Close() + + err = session.RebuildContainer(context.Background(), tt.fullRebuild) + if err != nil { + t.Fatalf("rebuilding codespace via mock session: %v", err) + } + + if requestCount == 0 { + t.Fatalf("no requests were made") + } } } From 647ba727f1df6e3c5faee2b6affb4f63ffdbb35b Mon Sep 17 00:00:00 2001 From: JP Ungaretti Date: Tue, 1 Nov 2022 21:10:58 +0000 Subject: [PATCH 2/4] Fix name of mock function --- pkg/liveshare/session_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/liveshare/session_test.go b/pkg/liveshare/session_test.go index b9dc91e9c..26f361a2f 100644 --- a/pkg/liveshare/session_test.go +++ b/pkg/liveshare/session_test.go @@ -416,12 +416,12 @@ func TestRebuild(t *testing.T) { for _, tt := range tests { requestCount := 0 - getSharedServers := func(conn *jsonrpc2.Conn, req *jsonrpc2.Request) (interface{}, error) { + rebuildContainer := func(conn *jsonrpc2.Conn, req *jsonrpc2.Request) (interface{}, error) { requestCount++ return true, nil } testServer, session, err := makeMockSession( - livesharetest.WithService(tt.rpcService, getSharedServers), + livesharetest.WithService(tt.rpcService, rebuildContainer), ) if err != nil { t.Fatalf("creating mock session: %v", err) From 66c4e7eca8f8ecd1678f47608e84d4ea91ce93f7 Mon Sep 17 00:00:00 2001 From: JP Ungaretti Date: Tue, 1 Nov 2022 21:45:56 +0000 Subject: [PATCH 3/4] Use Log and Error instead of Fatal --- pkg/liveshare/session_test.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/liveshare/session_test.go b/pkg/liveshare/session_test.go index 26f361a2f..4f7751032 100644 --- a/pkg/liveshare/session_test.go +++ b/pkg/liveshare/session_test.go @@ -415,6 +415,9 @@ func TestRebuild(t *testing.T) { } for _, tt := range tests { + t.Logf("RPC service: %s", tt.rpcService) + t.Logf("full rebuild: %t", tt.fullRebuild) + requestCount := 0 rebuildContainer := func(conn *jsonrpc2.Conn, req *jsonrpc2.Request) (interface{}, error) { requestCount++ @@ -424,17 +427,17 @@ func TestRebuild(t *testing.T) { livesharetest.WithService(tt.rpcService, rebuildContainer), ) if err != nil { - t.Fatalf("creating mock session: %v", err) + t.Errorf("creating mock session: %v", err) } defer testServer.Close() err = session.RebuildContainer(context.Background(), tt.fullRebuild) if err != nil { - t.Fatalf("rebuilding codespace via mock session: %v", err) + t.Errorf("rebuilding codespace via mock session: %v", err) } if requestCount == 0 { - t.Fatalf("no requests were made") + t.Errorf("no requests were made") } } } From 925edf38925e463611bdbffabbfc8e7ee1eab217 Mon Sep 17 00:00:00 2001 From: JP Ungaretti Date: Tue, 1 Nov 2022 22:35:39 +0000 Subject: [PATCH 4/4] Add long description --- pkg/cmd/codespace/rebuild.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/codespace/rebuild.go b/pkg/cmd/codespace/rebuild.go index 273627f07..246a6ebc4 100644 --- a/pkg/cmd/codespace/rebuild.go +++ b/pkg/cmd/codespace/rebuild.go @@ -15,14 +15,17 @@ func newRebuildCmd(app *App) *cobra.Command { rebuildCmd := &cobra.Command{ Use: "rebuild", Short: "Rebuild a codespace", - Args: cobra.NoArgs, + Long: `Rebuilding recreates your codespace. Your code and any current changes will be +preserved. Your codespace will be rebuilt using your working directory's +dev container. A full rebuild also removes cached Docker images.`, + Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { return app.Rebuild(cmd.Context(), codespace, fullRebuild) }, } - rebuildCmd.Flags().StringVarP(&codespace, "codespace", "c", "", "Name of the codespace") - rebuildCmd.Flags().BoolVar(&fullRebuild, "full", false, "Perform a full rebuild of the codespace") + rebuildCmd.Flags().StringVarP(&codespace, "codespace", "c", "", "name of the codespace") + rebuildCmd.Flags().BoolVar(&fullRebuild, "full", false, "perform a full rebuild") return rebuildCmd }