Job authoring: handling of task dependencies + some bugfixes

internal/manager/job_compilers/author.go

@@ -24,6 +24,7 @@ import (
 	"time"
 
 	"github.com/dop251/goja"
+	"github.com/google/uuid"
 	"github.com/rs/zerolog/log"
 	"gitlab.com/blender/flamenco-ng-poc/pkg/api"
 )
@@ -52,6 +53,11 @@ type JobSettings map[string]interface{}
 type JobMetadata map[string]string
 
 type AuthoredTask struct {
+	// Tasks already get their UUID in the authoring stage. This makes it simpler
+	// to store the dependencies, as the code doesn't have to worry about value
+	// vs. pointer semantics. Tasks can always be unambiguously referenced by
+	// their UUID.
+	UUID     string
 	Name     string
 	Type     string
 	Priority int
@@ -69,6 +75,7 @@ type AuthoredCommandParameters map[string]interface{}
 
 func (a *Author) Task(name string, taskType string) (*AuthoredTask, error) {
 	at := AuthoredTask{
+		uuid.New().String(),
 		name,
 		taskType,
 		50, // TODO: handle default priority somehow.

internal/manager/job_compilers/job_compilers_test.go

@@ -44,7 +44,7 @@ func exampleSubmittedJob() api.SubmittedJob {
 			"frames":                "1-10",
 			"images_or_video":       "images",
 			"output_file_extension": ".png",
-			"render_output":         "/render/sf/frames/scene123",
+			"render_output":         "/render/sprites/farm_output/promo/square_ellie/square_ellie.lighting_light_breakdown2/######",
 		}}
 	metadata := api.JobMetadata{
 		AdditionalProperties: map[string]string{
@@ -101,14 +101,15 @@ func TestSimpleBlenderRenderHappy(t *testing.T) {
 
 	settings := sj.Settings.AdditionalProperties
 
-	// Tasks should have been created to render the frames.
-	assert.Equal(t, 4, len(aj.Tasks))
+	// Tasks should have been created to render the frames: 1-3, 4-6, 7-9, 10, video-encoding
+	assert.Equal(t, 5, len(aj.Tasks))
 	t0 := aj.Tasks[0]
 	expectCliArgs := []interface{}{ // They are strings, but Goja doesn't know that and will produce an []interface{}.
-		"--render-output", "/render/sf__intermediate-2006-01-02_090405/frames",
+		"--render-output", "/render/sprites/farm_output/promo/square_ellie/square_ellie.lighting_light_breakdown2__intermediate-2006-01-02_090405/######",
 		"--render-format", settings["format"].(string),
 		"--render-frame", "1-3",
 	}
+	assert.NotEmpty(t, t0.UUID)
 	assert.Equal(t, "render-1-3", t0.Name)
 	assert.Equal(t, 1, len(t0.Commands))
 	assert.Equal(t, "blender-render", t0.Commands[0].Type)
@@ -117,4 +118,32 @@ func TestSimpleBlenderRenderHappy(t *testing.T) {
 		"blendfile": settings["filepath"].(string),
 		"args":      expectCliArgs,
 	}, t0.Commands[0].Parameters)
+
+	tVideo := aj.Tasks[4] // This should be a video encoding task
+	assert.NotEmpty(t, tVideo.UUID)
+	assert.Equal(t, "create-video", tVideo.Name)
+	assert.Equal(t, 1, len(tVideo.Commands))
+	assert.Equal(t, "create-video", tVideo.Commands[0].Type)
+	assert.EqualValues(t, AuthoredCommandParameters{
+		"input_files": "/render/sprites/farm_output/promo/square_ellie/square_ellie.lighting_light_breakdown2__intermediate-2006-01-02_090405/*.png",
+		"output_file": "/render/sprites/farm_output/promo/square_ellie/square_ellie.lighting_light_breakdown2__intermediate-2006-01-02_090405/scene123-1-10.mp4",
+		"fps":         int64(24),
+	}, tVideo.Commands[0].Parameters)
+
+	for index, task := range aj.Tasks {
+		if index == 0 {
+			continue
+		}
+		assert.NotEqual(t, t0.UUID, task.UUID, "Task UUIDs should be unique")
+	}
+
+	// Check dependencies
+	assert.Empty(t, aj.Tasks[0].Dependencies)
+	assert.Empty(t, aj.Tasks[1].Dependencies)
+	assert.Empty(t, aj.Tasks[2].Dependencies)
+	assert.Equal(t, 4, len(tVideo.Dependencies))
+	expectDeps := []*AuthoredTask{
+		&aj.Tasks[0], &aj.Tasks[1], &aj.Tasks[2], &aj.Tasks[3],
+	}
+	assert.Equal(t, expectDeps, tVideo.Dependencies)
 }

internal/manager/job_compilers/scripts/simple_blender_render.js

@@ -59,13 +59,16 @@ function compileJob(job) {
     // The render path contains a filename pattern, most likely '######' or
     // something similar. This has to be removed, so that we end up with
     // the directory that will contain the frames.
-    const renderOutput = path.dirname(settings.render_output);
+    const renderOutput = settings.render_output;
     const finalDir = path.dirname(renderOutput);
     const renderDir = intermediatePath(job, finalDir);
 
     const renderTasks = authorRenderTasks(settings, renderDir, renderOutput);
-    const videoTask = authorCreateVideoTask(renderTasks, renderDir);
+    const videoTask = authorCreateVideoTask(settings, renderDir);
 
+    for (const rt of renderTasks) {
+        job.addTask(rt);
+    }
     if (videoTask) {
         // If there is a video task, all other tasks have to be done first.
         for (const rt of renderTasks) {
@@ -73,9 +76,6 @@ function compileJob(job) {
         }
         job.addTask(videoTask);
     }
-    for (const rt of renderTasks) {
-        job.addTask(rt);
-    }
 }
 
 // Determine the intermediate render output path.
@@ -86,6 +86,7 @@ function intermediatePath(job, finalDir) {
 }
 
 function authorRenderTasks(settings, renderDir, renderOutput) {
+    print("authorRenderTasks(", renderDir, renderOutput, ")");
     let renderTasks = [];
     let chunks = frameChunker(settings.frames, settings.chunk_size);
     for (let chunk of chunks) {
@@ -107,9 +108,11 @@ function authorRenderTasks(settings, renderDir, renderOutput) {
 
 function authorCreateVideoTask(settings, renderDir) {
     if (ffmpegIncompatibleImageFormats.has(settings.format)) {
+        print("Not authoring video task, FFmpeg-incompatible render output")
         return;
     }
     if (!settings.fps || !settings.output_file_extension) {
+        print("Not authoring video task, no FPS or output file extension setting:", settings)
         return;
     }
 

internal/manager/persistence/jobs.go

@@ -50,6 +50,7 @@ type StringStringMap map[string]string
 
 type Task struct {
 	gorm.Model
+	UUID string `gorm:"type:char(36);not null;unique;index"`
 
 	Name     string `gorm:"type:varchar(64);not null"`
 	Type     string `gorm:"type:varchar(32);not null"`
@@ -137,6 +138,7 @@ func (db *DB) StoreAuthoredJob(ctx context.Context, authoredJob job_compilers.Au
 			dbTask := Task{
 				Name:     authoredTask.Name,
 				Type:     authoredTask.Type,
+				UUID:     authoredTask.UUID,
 				Job:      &dbJob,
 				Priority: authoredTask.Priority,
 				Status:   string(api.TaskStatusProcessing), // TODO: is this the right place to set the default status?

internal/manager/persistence/jobs_test.go

@@ -40,6 +40,7 @@ func TestStoreAuthoredJob(t *testing.T) {
 	task1 := job_compilers.AuthoredTask{
 		Name: "render-1-3",
 		Type: "blender",
+		UUID: "db1f5481-4ef5-4084-8571-8460c547ecaa",
 		Commands: []job_compilers.AuthoredCommand{
 			{
 				Type: "blender-render",
@@ -57,11 +58,13 @@ func TestStoreAuthoredJob(t *testing.T) {
 
 	task2 := task1
 	task2.Name = "render-4-6"
+	task2.UUID = "d75ac779-151b-4bc2-b8f1-d153a9c4ac69"
 	task2.Commands[0].Parameters["frames"] = "4-6"
 
 	task3 := job_compilers.AuthoredTask{
 		Name: "preview-video",
 		Type: "ffmpeg",
+		UUID: "4915fb05-72f5-463e-a2f4-7efdb2584a1e",
 		Commands: []job_compilers.AuthoredCommand{
 			{
 				Type: "merge-frames-to-video",