WIP: Allow jobs to be submitted in paused status #104318

David Zhang · 2024-06-20T01:41:35+02:00

David Zhang commented

2024-06-20 01:41:35 +02:00

Overview
This pull request implements Issue #99439 Allow jobs to be submitted in paused status and builds upon PR #104313 Support pausing jobs.

Deliverables

A new option on the addon side that allows users to choose whether they would like the job to be submitted in paused status before submission to Flamenco Manager.
Updates in relevant API implementation.
If the user does wish to submit a job in paused status, the job shows up correctly in the web interface.

**Overview** This pull request implements [Issue #99439 Allow jobs to be submitted in paused status](https://projects.blender.org/studio/flamenco/issues/99439) and builds upon [PR #104313 Support pausing jobs](https://projects.blender.org/studio/flamenco/pulls/104313). **Deliverables** - A new option on the addon side that allows users to choose whether they would like the job to be submitted in `paused` status before submission to Flamenco Manager. - Updates in relevant API implementation. - If the user does wish to submit a job in `paused` status, the job shows up correctly in the web interface.

David Zhang added 22 commits 2024-06-20 01:41:39 +02:00

Manager: draft task and job status transition; regenerated openapi with pause-requested status; start frontend work ec540e47a7

Manager: basic test case for paused status with all queued tasks 562f77bd74

Manager: leave out updateJobOnTaskStatusPaused 716f86c0f4

Manager: taskStatusesToPause should include soft-failed e30832aab4

Manager: improve error handling of pauseTasks() a84097e59f

Webapp: interactive Pause Job button ba454cd4a0

Manager: improve state transition logic 050c779995

Manager: cancelling tasks should not cause a pause-requested job to be cancelled 71c7715eab

Merge remote-tracking branch 'official/main' into paused-job-status 526e0f98fb

Manager: passing TestTaskStatusChangeActiveToCompleted 0e523a9b31

Manager: fix manager warning 1c35617cb1

Merge remote-tracking branch 'official/main' into paused-job-status ac5627e57a

Manager: pass TestTaskStatusChangeQueuedToFailed and TestTaskStatusChangeCancelSingleTask c3af84f6a6

Manager: disallow task cancellation when the job has pause-requested status c32a591e55

Manager: keep the newline between built-in packages and 3rd party packages 5a4e225a6b

Manager: reorder updateJobOnTaskStatusCanceled logic 4b4511f9b6

Webapp: end comment with a period c13cc884ce

Manager: pauseTasks keeps cancelled tasks as cancelled a569a1ce00

Webapp: update canCancel to properly expose bugs 05a1388106

Manager: correct logger message and reorder logic 01754e9e3c

Manager: reorder logic d510eb390c

OAPI: add "submit_as_paused" field to SubmittedJob 7835cb8c6d

David Zhang added 1 commit 2024-06-20 20:57:37 +02:00

OAPI: Regenerate code 86ddeb50e9

David Zhang added 4 commits 2024-06-20 21:59:49 +02:00

Manager: wrap job pause condition inside a function 8f067da901

Manager: all original test cases passed 009ad23da1

Manager: add more test cases for paused status 484f746f92

Merge remote-tracking branch 'origin/paused-job-status' into submit-as-paused 3f53a9ad2a

David Zhang added 1 commit 2024-06-22 15:04:27 +02:00

OAPI: change "submit_as_paused" to "initial_status" 56fdf15e2b

David Zhang added 1 commit 2024-06-22 15:05:54 +02:00

OAPI: Regenerate code 5a242095d9

David Zhang added 1 commit 2024-06-22 15:15:07 +02:00

OAPI: make initial_status refer to existing JobStatus definition 578ffb1446

David Zhang added 1 commit 2024-06-22 15:15:30 +02:00

OAPI: Regenerate code 9aecbbb95c

David Zhang added 1 commit 2024-06-22 16:39:22 +02:00

Manager: backend for submitting job with initial status e6bc9cd061

Sybren A. Stüvel reviewed 2024-06-26 17:43:59 +02:00

Sybren A. Stüvel left a comment

Deliverables

A new option on the addon side that allows users to choose whether they would like the job to be submitted in paused status before submission to Flamenco Manager.

It's better to keep the add-on out of this PR, and focus only on the API side.

Updates in relevant API implementation.

If the user does wish to submit a job in paused status, the job shows up correctly in the web interface.

These are good to have in this PR.

This was just a partial review, I haven't actually tried the code yet. I'll do that once #104313 lands and we can disentangle the two PRs.

> **Deliverables** > - A new option on the addon side that allows users to choose whether they would like the job to be submitted in `paused` status before submission to Flamenco Manager. It's better to keep the add-on out of this PR, and focus only on the API side. > - Updates in relevant API implementation. > - If the user does wish to submit a job in `paused` status, the job shows up correctly in the web interface. These are good to have in this PR. This was just a partial review, I haven't actually tried the code yet. I'll do that once #104313 lands and we can disentangle the two PRs.

debug-job-echo.sh

						
				@ -20,2 +20,3 @@

				  "type": "echo-sleep-test",

				  "submitter_platform": "manager"

				  "submitter_platform": "manager",

				  "initial_status": "paused"

Sybren A. Stüvel commented

2024-06-26 17:37:07 +02:00

Change this to queued so that the semantics of the script don't change.

For these kind of "I need a slightly different script" cases, I ususally just make a copy, then add its filename to .git/info/exclude so I don't accidentally commit it.

Change this to `queued` so that the semantics of the script don't change. For these kind of "I need a slightly different script" cases, I ususally just make a copy, then add its filename to `.git/info/exclude` so I don't accidentally commit it.

go.mod

						
				@ -59,6 +59,7 @@ require (

					github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec // indirect

					github.com/valyala/bytebufferpool v1.0.0 // indirect

					github.com/valyala/fasttemplate v1.2.1 // indirect

					golang.org/toolchain v0.0.1-go1.9rc2.windows-amd64 // indirect

Sybren A. Stüvel commented

2024-06-26 17:38:08 +02:00

How did this new dependency get added?

internal/manager/api_impl/jobs.go

						
				@ -92,7 +92,9 @@ func (f *Flamenco) SubmitJob(e echo.Context) error {

					logger = logger.With().Str("job_id", authoredJob.JobID).Logger()

					// TODO: check whether this job should be queued immediately or start paused.

Sybren A. Stüvel commented

2024-06-26 17:39:15 +02:00

This TODO can now be removed.

internal/manager/job_compilers/job_compilers.go

						
				@ -132,2 +132,4 @@

					}

					if sj.InitialStatus != nil {

						aj.Status = *sj.InitialStatus

Sybren A. Stüvel commented

2024-06-26 17:41:27 +02:00

What's the reason to handle this here, rather than in func (f *Flamenco) SubmitJob(e echo.Context) error? I think it's better to keep the status as under-construction while the job is still under construction, and then let it go to its initial status afterwards (like the current under-construction → queued transition).

What's the reason to handle this here, rather than in `func (f *Flamenco) SubmitJob(e echo.Context) error`? I think it's better to keep the status as `under-construction` while the job is still under construction, and then let it go to its initial status afterwards (like the current `under-construction` → `queued` transition).