studio/flamenco
studio/flamenco
12
30
Fork 38
Code Issues 62 Pull Requests 4 Releases Wiki Activity

Worker: check BLENDER_CMD environment variable (for multi-GPU Eevee rendering) #104193

Open
MKRelax wants to merge 9 commits from MKRelax/flamenco:worker-use-blender-from-env into main
pull from: MKRelax/flamenco:worker-use-blender-from-env
merge into: studio:main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
Conversation 12 Commits 9 Files Changed 11 +3 -1
1 changed files with 3 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit dc1077c4cf - Show all commits

4
internal/worker/command_blender.go
View File

@ -6,6 +6,7 @@ package worker
import ( import (
"context" "context"
"fmt"
"os/exec" "os/exec"
"regexp" "regexp"
"sync" "sync"
@ -88,7 +89,8 @@ func (ce *CommandExecutor) cmdBlenderRenderCommand(
// No directory path given. Check that the executable is set in an // No directory path given. Check that the executable is set in an
// environment variable or can be found on the path. // environment variable or can be found on the path.
if path := find_blender.EnvironmentVariable(); path != "" { if path := find_blender.EnvironmentVariable(); path != "" {
logger.Info().Str("path", path).Msg("found Blender in environment") msg := fmt.Sprintf("using blender from %s", find_blender.BlenderPathEnvVariable)
logger.Info().Str("path", path).Msg(msg)
Sybren A. Stüvel commented 2023-03-03 12:43:40 +01:00
Outdated
Review
Copy Link

I wouldn't say "found Blender" here, because the path wasn't checked. Blender may not even exist there.

Maybe find_blender.EnvironmentVariable() can return two strings, (name, path) where name is the name of the environment variable, and path is the actual value. That way you can log here "using Blender from {environment variable name}".

I wouldn't say "found Blender" here, because the path wasn't checked. Blender may not even exist there. Maybe `find_blender.EnvironmentVariable()` can return two strings, `(name, path)` where `name` is the name of the environment variable, and `path` is the actual value. That way you can log here "using Blender from {environment variable name}".
MKRelax commented 2023-03-04 02:59:10 +01:00
Outdated
Review
Copy Link

I did change the log message to "using Blender from FLAMENCO_BLENDER_PATH".

However, since the name of the variable is (now) a constant I think there is no need to return it from find_blender.EnvironmentVariable(). Most code would not be interested in it anyway and probably ignore it using a _ variable. So the log message simply uses the constant.

I did change the log message to "using Blender from FLAMENCO_BLENDER_PATH". However, since the name of the variable is (now) a constant I think there is no need to return it from `find_blender.EnvironmentVariable()`. Most code would not be interested in it anyway and probably ignore it using a `_` variable. So the log message simply uses the constant.
Sybren A. Stüvel commented 2023-03-06 10:50:45 +01:00
Outdated
Review
Copy Link

I think it's still good to return the variable name. That way you get less coupling between the different parts of the code, making things more maintainable.

I think it's still good to return the variable name. That way you get less coupling between the different parts of the code, making things more maintainable.
parameters.exe = path parameters.exe = path
} else if _, err := exec.LookPath(parameters.exe); err != nil { } else if _, err := exec.LookPath(parameters.exe); err != nil {
// Attempt a platform-specific way to find which Blender executable to // Attempt a platform-specific way to find which Blender executable to