From 6baa132c43ef2882f211df6712d2110ac51a723a Mon Sep 17 00:00:00 2001 From: Mateus Abelli Date: Mon, 9 Sep 2024 11:10:53 +0200 Subject: [PATCH] Manager: allow setup to finish without Blender MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add an option to the setup assistant to skip configuring the path to Blender. It will just use the `default` option, which causes the Workers to try and find Blender on their own. Fixes #104306 Reviewed-on: https://projects.blender.org/studio/flamenco/pulls/104306 Reviewed-by: Sybren A. Stüvel --- internal/manager/api_impl/meta.go | 52 +++++++++++++++++++++--- internal/manager/api_impl/meta_test.go | 21 ++++++++++ web/app/src/views/SetupAssistantView.vue | 47 ++++++++++++--------- 3 files changed, 94 insertions(+), 26 deletions(-) diff --git a/internal/manager/api_impl/meta.go b/internal/manager/api_impl/meta.go index 000bd5de..491cbb23 100644 --- a/internal/manager/api_impl/meta.go +++ b/internal/manager/api_impl/meta.go @@ -21,6 +21,14 @@ import ( "projects.blender.org/studio/flamenco/pkg/api" ) +var ( + ErrSetupConfigUnusableSource = errors.New("sources should not have the 'is_usable' field set to false") + ErrSetupConfigEmptyStorageLocation = errors.New("'storageLocation' field must not be empty") + ErrSetupConfigEmptyPath = errors.New("'path' field must not be empty while using the 'file_association' source") + ErrSetupConfigEmptyPathOrInput = errors.New("'path' or 'input' fields must not be empty while using the 'input_path' or 'path_envvar' sources") + ErrSetupConfigEmptySource = errors.New("'source' field must not be empty") +) + func (f *Flamenco) GetVersion(e echo.Context) error { return e.JSON(http.StatusOK, api.FlamencoVersion{ Version: appinfo.ExtendedVersion(), @@ -265,11 +273,9 @@ func (f *Flamenco) SaveSetupAssistantConfig(e echo.Context) error { logger = logger.With().Interface("config", setupAssistantCfg).Logger() - if setupAssistantCfg.StorageLocation == "" || - !setupAssistantCfg.BlenderExecutable.IsUsable || - setupAssistantCfg.BlenderExecutable.Path == "" { - logger.Warn().Msg("setup assistant: configuration is incomplete, unable to accept") - return sendAPIError(e, http.StatusBadRequest, "configuration is incomplete") + if err := checkSetupAssistantConfig(setupAssistantCfg); err != nil { + logger.Error().AnErr("cause", err).Msg("setup assistant: configuration is incomplete") + return sendAPIError(e, http.StatusBadRequest, "configuration is incomplete: %v", err) } conf := f.config.Get() @@ -277,7 +283,7 @@ func (f *Flamenco) SaveSetupAssistantConfig(e echo.Context) error { var executable string switch setupAssistantCfg.BlenderExecutable.Source { - case api.BlenderPathSourceFileAssociation: + case api.BlenderPathSourceFileAssociation, api.BlenderPathSourceDefault: // The Worker will try to use the file association when the command is set // to the string "blender". executable = "blender" @@ -336,3 +342,37 @@ func flamencoManagerDir() (string, error) { func commandNeedsQuoting(cmd string) bool { return strings.ContainsAny(cmd, "\n\t;()") } + +func checkSetupAssistantConfig(config api.SetupAssistantConfig) error { + if config.StorageLocation == "" { + return ErrSetupConfigEmptyStorageLocation + } + + if !config.BlenderExecutable.IsUsable { + return ErrSetupConfigUnusableSource + } + + switch config.BlenderExecutable.Source { + case api.BlenderPathSourceDefault: + return nil + + case api.BlenderPathSourceFileAssociation: + if config.BlenderExecutable.Path == "" { + return ErrSetupConfigEmptyPath + } + + case api.BlenderPathSourceInputPath, api.BlenderPathSourcePathEnvvar: + if config.BlenderExecutable.Path == "" || + config.BlenderExecutable.Input == "" { + return ErrSetupConfigEmptyPathOrInput + } + + case "": + return ErrSetupConfigEmptySource + + default: + return fmt.Errorf("unknown 'source' field value: %v", config.BlenderExecutable.Source) + } + + return nil +} diff --git a/internal/manager/api_impl/meta_test.go b/internal/manager/api_impl/meta_test.go index 78581804..491bb700 100644 --- a/internal/manager/api_impl/meta_test.go +++ b/internal/manager/api_impl/meta_test.go @@ -363,6 +363,27 @@ func TestSaveSetupAssistantConfig(t *testing.T) { assert.Equal(t, expectBlenderVar, savedConfig.Variables["blender"]) assert.Equal(t, defaultBlenderArgsVar, savedConfig.Variables["blenderArgs"]) } + + // Test situation where adding a blender executable was skipped. + { + savedConfig := doTest(api.SetupAssistantConfig{ + StorageLocation: mf.tempdir, + BlenderExecutable: api.BlenderPathCheckResult{ + IsUsable: true, + Source: api.BlenderPathSourceDefault, + }, + }) + assert.Equal(t, mf.tempdir, savedConfig.SharedStoragePath) + expectBlenderVar := config.Variable{ + Values: config.VariableValues{ + {Platform: "linux", Value: "blender"}, + {Platform: "windows", Value: "blender"}, + {Platform: "darwin", Value: "blender"}, + }, + } + assert.Equal(t, expectBlenderVar, savedConfig.Variables["blender"]) + assert.Equal(t, defaultBlenderArgsVar, savedConfig.Variables["blenderArgs"]) + } } func metaTestFixtures(t *testing.T) (mockedFlamenco, func()) { diff --git a/web/app/src/views/SetupAssistantView.vue b/web/app/src/views/SetupAssistantView.vue index 869ed064..7e9dc450 100644 --- a/web/app/src/views/SetupAssistantView.vue +++ b/web/app/src/views/SetupAssistantView.vue @@ -114,7 +114,7 @@

Choose how a Worker should invoke the Blender command when performing a task:

-
+
+ +
- -
- - -

Checking...

-

- {{ blenderExeCheckResult.cause }} -

-
{{ selectedBlender.path }}" +
+ You have chosen to skip adding a blender path. +

@@ -291,7 +288,8 @@ export default { sourceLabels: { file_association: 'Blender that runs when you double-click a .blend file:', path_envvar: 'Blender found on the $PATH environment:', - input_path: 'Another Blender executable:', + input_path: 'Specify a Blender executable:', + default: 'Skip, let the Workers use whatever Blender is available.', }, isConfirming: false, isConfirmed: false, @@ -324,6 +322,15 @@ export default { blenderFromInputPath() { return this.allBlenders.find((b) => b.source === 'input_path'); }, + blenderFromDefaultSource() { + return { + input: '', + path: '', + source: 'default', + is_usable: true, + cause: '', + }; + }, setupConfirmIsClickable() { if (this.isConfirming || this.isConfirmed) { return false;