Cuesubmit jobs from config file #1284

KernAttila · 2023-03-29T00:47:46Z

Link the Issue(s) this Pull Request is related to.
#1275

Summarize your change.

We are now able to declare jobs from the configuration file (see cuesubmit/cuesubmit_config.example.yaml)
We can nest configuration files and use env variables.
The flag order is respected
Widgets are constructed according to each flag type:
The yaml syntax allows solo flags, pycue tokens, mandatory flags, no flags arguments and browsable fields.
Invalid text fields are red from the start
Added some unittests, would love some feedback on those.
Each job type can have limits and services preset.

Note

This PR should be merged after :

Question

Should we deprecate (or remove) the hardcoded commands ?
Are you ok with the commands syntax in the example yaml file ?

Screenshots

linux-foundation-easycla · 2023-05-26T12:55:22Z

The committers listed above are authorized under a signed CLA.

✅ login: KernAttila / name: Kern Attila GERMAIN (2c2bf7a, f26d5ca, f873d55, 3ccea48, 14a0385, 0d375d2, dfb220d, b942729, 490bc8d, 1f86dc2, d761fac, 72adc3d, 61fcd8d, da98b04, eef22bd, cfb8fe3, 03c08b6, 4ac65a7, a254c9a, ecc71a4, 3e60389, 6feddc3, fe1a6d6, b9aa0e2, 46ea2d8, 7e6846c, ed56fc5, 0a9501c, 195091f, 47ac2ef, 5ae3145, 108670a, cb08c88, 5d774b3, 3e207cb, fd924ad, e152336, a8d563c, 3bc307c, 17bc8d2, 288adda, 7ea1562, a84707a, ff2bdda, 959613d, 8c5a6ed, 161f17b, 60551e8, 4ef105d, 5556f11, fe4d549, 1d73834, 0076500, f385cc0, 5b6c204, fb7967d, 23075d4, 743e180, 9275e3a, 1bb0b7a, 06368cc, fb91f2f, 82e98f1, c4cd63e, 47daaa6, 04d3a06, 43ca5e9, 73a66eb, af4f5dc, 01d3fb2, 261db2f, 936f368, 4b97913, b3408cc, 726a374, 34f2fea, 8f82fa5, 7e3b181, c620039, 3c0a053, 731bf80, 9629f6c, 69d118f, 3b028b9, f7062af, fda4ab8, 02fac67, 134fd5d)
✅ login: lithorus / name: Jimmy Christensen (ab69f7f)

DiegoTavares · 2024-05-03T23:25:00Z

This branch is failing on CLA for some reason. @KernAttila I know it has been a while, but can you try following the CLA process again?

DiegoTavares · 2024-08-07T15:08:35Z

It looks like some errors emerged after merging master into this branch. Unit tests are failing with:

AttributeError: module 'cuesubmit.Constants' has no attribute 'RENDER_CMDS'

KernAttila · 2024-08-07T15:28:18Z

@DiegoTavares no worries, I'm currently fixing those. It's a bit hard to come back to this code after so long ^^ but we'll get there !
I will let you know when things are ready.

…p section

…b `config_file` + error handling

… config file

…ttingsWidget`

…rom-config-file

…during CI)

KernAttila · 2024-09-21T01:33:25Z

I tested a lot of scenarios, everything seems up to expectation with this feature.
There's an additional option in the config file: being able to have services and limits presets for each job type.
There is also more tooltips in the UI to help understand each widget.

IMPORTANT NOTE: I added the "Override Cores" feature in this branch as well, it goes with #1296 (rqd) and #1313 (cuebot)
I could move it to another branch if you prefer.

KernAttila · 2024-09-23T21:36:15Z

pyoutline/outline/backend/cue.py

+        cores = None
+        if layer.get_arg("overrideCores"):
+            logger.info("%s is set to override service cores.", layer.get_name())
+            if layer.is_arg_set("cores") and layer.is_arg_set("threads"):
                logger.warning("%s has both cores and threads. Use cores.", layer.get_name())
-            sub_element(spec_layer, "cores", "%0.1f" % (layer.get_arg("cores")))
-        elif layer.get_arg("threads"):
-            sub_element(spec_layer, "cores", "%0.1f" % (layer.get_arg("threads")))
+            cores = "%0.1f" % float(layer.get_arg("cores") or layer.get_arg("threads", 0))
+            sub_element(spec_layer, "cores", cores)
+        else:
+            logger.warning("%s will use service cores.", layer.get_name())


Looking back at this code, it introduces a mandatory argument "overrideCores" that will disrupt existing implementations relying only on pycue.
Another approach could be to test if "cores" is not None, meaning we explicitly override the job's cores value over the service one, thus making current codebases retro-compatible with this feature.
If "cores" is None, then we let the selected service handle that value.
What do you think @DiegoTavares @bcipriano @lithorus ?

Yeah, this change would break our pipeline today. I'd rather have it implemented as you suggested to make it retro-compatible.

The change is done and tested.
overrideCores stays in cuesubmit.

DiegoTavares · 2024-09-25T16:13:57Z

I tested a lot of scenarios, everything seems up to expectation with this feature. There's an additional option in the config file: being able to have services and limits presets for each job type. There is also more tooltips in the UI to help understand each widget.

IMPORTANT NOTE: I added the "Override Cores" feature in this branch as well, it goes with #1296 (rqd) and #1313 (cuebot) I could move it to another branch if you prefer.

I don't see a problem in keeping #1296 and #1313 on this PR, as long as they get approved and merged beforehand

DiegoTavares

Aside from the suggested change, this MR is ready to be merged.

DiegoTavares · 2024-09-25T16:18:44Z

pyoutline/outline/backend/cue.py

+        cores = None
+        if layer.get_arg("overrideCores"):
+            logger.info("%s is set to override service cores.", layer.get_name())
+            if layer.is_arg_set("cores") and layer.is_arg_set("threads"):
                logger.warning("%s has both cores and threads. Use cores.", layer.get_name())
-            sub_element(spec_layer, "cores", "%0.1f" % (layer.get_arg("cores")))
-        elif layer.get_arg("threads"):
-            sub_element(spec_layer, "cores", "%0.1f" % (layer.get_arg("threads")))
+            cores = "%0.1f" % float(layer.get_arg("cores") or layer.get_arg("threads", 0))
+            sub_element(spec_layer, "cores", cores)
+        else:
+            logger.warning("%s will use service cores.", layer.get_name())


Yeah, this change would break our pipeline today. I'd rather have it implemented as you suggested to make it retro-compatible.

**Link the Issue(s) this Pull Request is related to.** Fixes #1297 **Summarize your change.** As in many render engines, we should be able to set a negative core requirement. minCores=8 > reserve 8 cores minCores=0 > reserve all cores minCores=-2 > reserve all cores minus 2 This PR addresses this feature by handling negative core requests. Cuebot will try to match this number against the number of cores on each host. The frame will be booked only if all cores are available in this scenario. If the host is busy (even slightly), the frame is **not** booked, to avoid filling the remaining cores. **Testing** I would need some guidance to create proper tests for cuebot. **Screenshot** ![negative_cores](https://github.com/AcademySoftwareFoundation/OpenCue/assets/5556461/d9c4400c-824a-40cc-9ba9-2f76a3fd8ceb) Update: There is now a "ALL" text for zero cores, or "ALL (-2)" for negative cores reservation. ![core_reservation](https://github.com/user-attachments/assets/88802b15-3ccd-4cb5-90b7-58e532523ae6) (cuesubmit feature in another PR #1284) --------- Signed-off-by: Kern Attila GERMAIN <[email protected]>

…g if "cores" is not None. If it has an explicit value (like in existing implementations), this means we want to override the value, which is the default behavior. Otherwise, we let the server assign the value of selected service.

…s" value is None.

…uesubmit.

Signed-off-by: Kern Attila GERMAIN <[email protected]>

… the args are set, because checking their value would be False for 0 (all cores)

…Wolf-Pipeline/OpenCue into cuesubmit-jobs-from-config-file

KernAttila marked this pull request as ready for review April 4, 2023 14:08

KernAttila requested review from bcipriano, gregdenton, jrray, smith1511, larsbijl, DiegoTavares, IdrisMiles and splhack as code owners April 4, 2023 14:08

KernAttila force-pushed the cuesubmit-jobs-from-config-file branch from 9e85fe0 to 6cfd785 Compare May 29, 2023 09:16

This was referenced Jul 17, 2024

Cuesubmit browse widget #1281

Closed

Cuesubmit jobs from config file with dynamic widgets #1425

Merged

KernAttila requested a review from ramonfigueiredo as a code owner August 7, 2024 14:08

KernAttila force-pushed the cuesubmit-jobs-from-config-file branch from 47010d1 to 91483f0 Compare August 7, 2024 14:20

KernAttila added 12 commits August 8, 2024 12:30

doc!: update cuesubmit example config file with new jobs syntax + hel…

7ea1562

…p section

doc!: add a cuesubmit isolated example config file

14a0385

feat: add notEmptyString validator

d761fac

feat: extract yaml loading to a function and add one to expand any su…

72adc3d

…b `config_file` + error handling

feat: load RENDER_CMDS in Constants

959613d

feat: add regex to get opencue tokens

f26d5ca

feat: add regex to parse options from the cuesubmit config file

195091f

feat: add convertCommandOptions() to parse command options from the…

0d375d2

… config file

feat: Append jobs from the config file to JobTypes

da98b04

feat: return JobTypes.types dynamically

47ac2ef

feat: Associate DynamicSettingsWidget to all jobs from the config file

6feddc3

feat: Convert custom commands options and pass them to the `DynamicSe…

03c08b6

…ttingsWidget`

KernAttila added 9 commits September 21, 2024 01:16

chore: DynamicSettingsWidget: manage sizePolicy to avoid it expanding.

23075d4

feat: add option to override min cores.

4b97913

chore: add tooltips and re-order UI to gather related widgets.

43ca5e9

feat: add overrideCores option during job submission.

f385cc0

Merge branch 'AcademySoftwareFoundation:master' into cuesubmit-jobs-f…

9629f6c

…rom-config-file

chores: fix pylint issues

47daaa6

test: update test with the override setting

fb91f2f

fix: forgot overrideCore argument in the buildFactory.

82e98f1

test: handle connection error to pass the tests (no cuebot available …

06368cc

…during CI)

KernAttila mentioned this pull request Sep 21, 2024

Cuebot reserve all cores #1313

Merged

KernAttila commented Sep 23, 2024

View reviewed changes

DiegoTavares requested changes Sep 25, 2024

View reviewed changes

KernAttila added 15 commits September 29, 2024 00:12

fix: revert to default behavior, we do not override only if the "core…

731bf80

…s" value is None.

fix: revert to default behavior, we do not override only if the "core…

f7062af

…s" value is None.

fix: only send "cores" to pyoutline, "overrideCores" is only set in c…

01d3fb2

…uesubmit.

fix: better UI scaling

b3408cc

Merge branch 'master' into cuesubmit-jobs-from-config-file

34f2fea

Signed-off-by: Kern Attila GERMAIN <[email protected]>

fix: lint line too long

1bb0b7a

fix: Make sure we use threads for backward compatibility. We check if…

02fac67

… the args are set, because checking their value would be False for 0 (all cores)

Merge branch 'cuesubmit-jobs-from-config-file' of https://github.com/…

3b028b9

…Wolf-Pipeline/OpenCue into cuesubmit-jobs-from-config-file

fix: cast str to float

936f368

feat: add opencue icon to cuesubmit window

73a66eb

fix: add overrideCores attribute in LayerData.toDict()

134fd5d

test: add overrideCores param

261db2f

Merge branch 'master' into cuesubmit-jobs-from-config-file

fda4ab8

Merge branch 'master' into cuesubmit-jobs-from-config-file

7e3b181

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cuesubmit jobs from config file #1284

Cuesubmit jobs from config file #1284

KernAttila commented Mar 29, 2023 •

edited

Loading

linux-foundation-easycla bot commented May 26, 2023 •

edited

Loading

DiegoTavares commented May 3, 2024

DiegoTavares commented Aug 7, 2024

KernAttila commented Aug 7, 2024

KernAttila commented Sep 21, 2024

KernAttila Sep 23, 2024

DiegoTavares Sep 25, 2024

KernAttila Sep 29, 2024

DiegoTavares commented Sep 25, 2024

DiegoTavares left a comment

DiegoTavares Sep 25, 2024

Cuesubmit jobs from config file #1284

Are you sure you want to change the base?

Cuesubmit jobs from config file #1284

Conversation

KernAttila commented Mar 29, 2023 • edited Loading

linux-foundation-easycla bot commented May 26, 2023 • edited Loading

DiegoTavares commented May 3, 2024

DiegoTavares commented Aug 7, 2024

KernAttila commented Aug 7, 2024

KernAttila commented Sep 21, 2024

KernAttila Sep 23, 2024

Choose a reason for hiding this comment

DiegoTavares Sep 25, 2024

Choose a reason for hiding this comment

KernAttila Sep 29, 2024

Choose a reason for hiding this comment

DiegoTavares commented Sep 25, 2024

DiegoTavares left a comment

Choose a reason for hiding this comment

DiegoTavares Sep 25, 2024

Choose a reason for hiding this comment

KernAttila commented Mar 29, 2023 •

edited

Loading

linux-foundation-easycla bot commented May 26, 2023 •

edited

Loading