diff --git a/CHANGELOG.md b/CHANGELOG.md index 023598b2..cd00cc80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ bugs in actually-released versions. - Bump the bundled Blender Asset Tracer (BAT) to version 1.15. - Increase preview image file size from 10 MB to 25 MB. Even though the Worker can down-scale render output before sending to the Manager as preview, they could still be larger than the limit of 10 MB. - Fix a crash of the Manager when using an invalid frame range (`1 10` for example, instead of `1-10` or `1,10`) +- Make it possible to delete jobs. The job and its tasks are removed from Flamenco, including last-rendered images and logs. The input files (i.e. the to-be-rendered blend files and their dependencies) will only be removed if [the Shaman system](https://flamenco.blender.org/usage/shared-storage/shaman/) was used AND if the job was submitted with Flamenco 3.2 or newer. + ## 3.1 - released 2022-10-18 diff --git a/internal/manager/api_impl/interfaces.go b/internal/manager/api_impl/interfaces.go index bfa778df..b34cbe4d 100644 --- a/internal/manager/api_impl/interfaces.go +++ b/internal/manager/api_impl/interfaces.go @@ -223,6 +223,7 @@ var _ WorkerSleepScheduler = (*sleep_scheduler.SleepScheduler)(nil) type JobDeleter interface { QueueJobDeletion(ctx context.Context, job *persistence.Job) error + WhatWouldBeDeleted(job *persistence.Job) api.JobDeletionInfo } var _ JobDeleter = (*job_deleter.Service)(nil) diff --git a/internal/manager/api_impl/jobs.go b/internal/manager/api_impl/jobs.go index 97407072..cf78eb78 100644 --- a/internal/manager/api_impl/jobs.go +++ b/internal/manager/api_impl/jobs.go @@ -194,6 +194,26 @@ func (f *Flamenco) DeleteJob(e echo.Context, jobID string) error { } } +func (f *Flamenco) DeleteJobWhatWouldItDo(e echo.Context, jobID string) error { + logger := requestLogger(e).With(). + Str("job", jobID). + Logger() + + dbJob, err := f.fetchJob(e, logger, jobID) + if dbJob == nil { + // f.fetchJob already sent a response. + return err + } + + logger = logger.With(). + Str("currentstatus", string(dbJob.Status)). + Logger() + logger.Info().Msg("checking what job deletion would do") + + deletionInfo := f.jobDeleter.WhatWouldBeDeleted(dbJob) + return e.JSON(http.StatusOK, deletionInfo) +} + // SetJobStatus is used by the web interface to change a job's status. func (f *Flamenco) SetJobStatus(e echo.Context, jobID string) error { logger := requestLogger(e).With(). diff --git a/internal/manager/job_deleter/job_deleter.go b/internal/manager/job_deleter/job_deleter.go index 8953c23c..9fb7e2d0 100644 --- a/internal/manager/job_deleter/job_deleter.go +++ b/internal/manager/job_deleter/job_deleter.go @@ -17,6 +17,8 @@ import ( "time" "git.blender.org/flamenco/internal/manager/persistence" + "git.blender.org/flamenco/internal/manager/webupdates" + "git.blender.org/flamenco/pkg/api" "git.blender.org/flamenco/pkg/shaman" "github.com/rs/zerolog" "github.com/rs/zerolog/log" @@ -67,6 +69,10 @@ func (s *Service) QueueJobDeletion(ctx context.Context, job *persistence.Job) er return fmt.Errorf("requesting job deletion: %w", err) } + // Broadcast that this job was queued for deleted. + jobUpdate := webupdates.NewJobUpdate(job) + s.changeBroadcaster.BroadcastJobUpdate(jobUpdate) + // Let the Run() goroutine know this job is ready for deletion. select { case s.queue <- job.UUID: @@ -77,6 +83,15 @@ func (s *Service) QueueJobDeletion(ctx context.Context, job *persistence.Job) er return nil } +func (s *Service) WhatWouldBeDeleted(job *persistence.Job) api.JobDeletionInfo { + logger := log.With().Str("job", job.UUID).Logger() + logger.Info().Msg("job deleter: checking what job deletion would do") + + return api.JobDeletionInfo{ + ShamanCheckout: s.canDeleteShamanCheckout(logger, job), + } +} + // Run processes the queue of deletion requests. It starts by building up a // queue of still-pending job deletions. func (s *Service) Run(ctx context.Context) { @@ -142,13 +157,39 @@ func (s *Service) deleteJob(ctx context.Context, jobUUID string) error { return err } - // TODO: broadcast that this job was deleted. + // Broadcast that this job was deleted. This only contains the UUID and the + // "was deleted" flag, because there's nothing else left. And I don't want to + // do a full database query for something we'll delete anyway. + wasDeleted := true + jobUpdate := api.SocketIOJobUpdate{ + Id: jobUUID, + WasDeleted: &wasDeleted, + } + s.changeBroadcaster.BroadcastJobUpdate(jobUpdate) logger.Info().Msg("job deleter: job removal complete") return nil } +func (s *Service) canDeleteShamanCheckout(logger zerolog.Logger, job *persistence.Job) bool { + // NOTE: Keep this logic and the deleteShamanCheckout() function in sync. + if !s.shaman.IsEnabled() { + logger.Debug().Msg("job deleter: Shaman not enabled, cannot delete job files") + return false + } + + checkoutID := job.Storage.ShamanCheckoutID + if checkoutID == "" { + logger.Debug().Msg("job deleter: job was not created with Shaman (or before Flamenco v3.2), cannot delete job files") + return false + } + + return true +} + func (s *Service) deleteShamanCheckout(ctx context.Context, logger zerolog.Logger, jobUUID string) error { + // NOTE: Keep this logic and the canDeleteShamanCheckout() function in sync. + if !s.shaman.IsEnabled() { logger.Debug().Msg("job deleter: Shaman not enabled, skipping job file deletion") return nil diff --git a/internal/manager/job_deleter/job_deleter_test.go b/internal/manager/job_deleter/job_deleter_test.go index a321dd84..4b479fd9 100644 --- a/internal/manager/job_deleter/job_deleter_test.go +++ b/internal/manager/job_deleter/job_deleter_test.go @@ -28,6 +28,8 @@ func TestQueueJobDeletion(t *testing.T) { s, finish, mocks := jobDeleterTestFixtures(t) defer finish() + mocks.broadcaster.EXPECT().BroadcastJobUpdate(gomock.Any()).Times(3) + job1 := &persistence.Job{UUID: "2f7d910f-08a6-4b0f-8ecb-b3946939ed1b"} mocks.persist.EXPECT().RequestJobDeletion(mocks.ctx, job1) assert.NoError(t, s.QueueJobDeletion(mocks.ctx, job1)) @@ -107,7 +109,7 @@ func TestDeleteJobWithoutShaman(t *testing.T) { // Mock that everything went OK. mocks.storage.EXPECT().RemoveJobStorage(mocks.ctx, jobUUID) mocks.persist.EXPECT().DeleteJob(mocks.ctx, jobUUID) - // TODO: mocks.broadcaster.EXPECT().BroadcastJobUpdate(...) + mocks.broadcaster.EXPECT().BroadcastJobUpdate(gomock.Any()) assert.NoError(t, s.deleteJob(mocks.ctx, jobUUID)) } @@ -158,7 +160,7 @@ func TestDeleteJobWithShaman(t *testing.T) { mocks.shaman.EXPECT().EraseCheckout(shamanCheckoutID) mocks.storage.EXPECT().RemoveJobStorage(mocks.ctx, jobUUID) mocks.persist.EXPECT().DeleteJob(mocks.ctx, jobUUID) - // TODO: mocks.broadcaster.EXPECT().BroadcastJobUpdate(...) + mocks.broadcaster.EXPECT().BroadcastJobUpdate(gomock.Any()) assert.NoError(t, s.deleteJob(mocks.ctx, jobUUID)) } diff --git a/internal/manager/persistence/jobs.go b/internal/manager/persistence/jobs.go index 5a414899..432a5b2e 100644 --- a/internal/manager/persistence/jobs.go +++ b/internal/manager/persistence/jobs.go @@ -216,6 +216,9 @@ func (db *DB) FetchJob(ctx context.Context, jobUUID string) (*Job, error) { if findResult.Error != nil { return nil, jobError(findResult.Error, "fetching job") } + if dbJob.ID == 0 { + return nil, ErrJobNotFound + } return &dbJob, nil } diff --git a/internal/manager/webupdates/job_updates.go b/internal/manager/webupdates/job_updates.go index 9e611f26..eef3ace7 100644 --- a/internal/manager/webupdates/job_updates.go +++ b/internal/manager/webupdates/job_updates.go @@ -21,6 +21,11 @@ func NewJobUpdate(job *persistence.Job) api.SocketIOJobUpdate { Type: job.JobType, Priority: job.Priority, } + + if job.DeleteRequestedAt.Valid { + jobUpdate.DeleteRequestedAt = &job.DeleteRequestedAt.Time + } + return jobUpdate } diff --git a/web/app/src/components/jobs/JobActionsBar.vue b/web/app/src/components/jobs/JobActionsBar.vue index b13952f4..8361c745 100644 --- a/web/app/src/components/jobs/JobActionsBar.vue +++ b/web/app/src/components/jobs/JobActionsBar.vue @@ -1,25 +1,52 @@ diff --git a/web/app/src/components/jobs/JobDetails.vue b/web/app/src/components/jobs/JobDetails.vue index 9b6dc2b7..5cfe6ca9 100644 --- a/web/app/src/components/jobs/JobDetails.vue +++ b/web/app/src/components/jobs/JobDetails.vue @@ -39,10 +39,12 @@
{{ jobData.status }}
Type
-
{{ jobType ? jobType.label : jobData.type }}
+
{{ jobType? jobType.label : jobData.type }}
Priority
-
+
+ +
Created
{{ datetime.relativeTime(jobData.created) }}
@@ -140,6 +142,8 @@ export default { }, watch: { jobData(newJobData) { + // This can be called when moving from "a job" to "no job", in which case there is no ID. + if (!newJobData || !newJobData.id) return; this._refreshJobSettings(newJobData); }, }, diff --git a/web/app/src/components/jobs/JobsTable.vue b/web/app/src/components/jobs/JobsTable.vue index 76cae56c..534e3806 100644 --- a/web/app/src/components/jobs/JobsTable.vue +++ b/web/app/src/components/jobs/JobsTable.vue @@ -1,7 +1,7 @@