Skip to content

Cuesubmit jobs from config file #1284

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 95 commits into
base: master
Choose a base branch
from
Open

Cuesubmit jobs from config file #1284

wants to merge 95 commits into from

Conversation

KernAttila
Copy link
Contributor

@KernAttila KernAttila commented Mar 29, 2023 •
edited
Loading

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

Question

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

Screenshots
update_Cuesubmit

@KernAttila KernAttila marked this pull request as ready for review April 4, 2023 14:08
@linux-foundation-easycla
Copy link

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

CLA Signed

The committers listed above are authorized under a signed CLA.

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

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?

This was referenced Jul 17, 2024
Closed
Merged
@KernAttila KernAttila force-pushed the cuesubmit-jobs-from-config-file branch from 47010d1 to 91483f0 Compare August 7, 2024 14:20
@DiegoTavares
Copy link
Collaborator

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
Copy link
Contributor Author

@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.

KernAttila added 12 commits August 8, 2024 12:30
@KernAttila
doc!: update cuesubmit example config file with new jobs syntax + hel…
7ea1562 
…p section
@KernAttila
doc!: add a cuesubmit isolated example config file
14a0385
@KernAttila
feat: add notEmptyString validator
d761fac
@KernAttila
feat: extract yaml loading to a function and add one to expand any su…
72adc3d 
…b `config_file` + error handling
@KernAttila
feat: load RENDER_CMDS in Constants
959613d
@KernAttila
feat: add regex to get opencue tokens
f26d5ca
@KernAttila
feat: add regex to parse options from the cuesubmit config file
195091f
@KernAttila
feat: add convertCommandOptions() to parse command options from the…
0d375d2 
… config file
@KernAttila
feat: Append jobs from the config file to JobTypes
da98b04
@KernAttila
feat: return JobTypes.types dynamically
47ac2ef
@KernAttila
feat: Associate DynamicSettingsWidget to all jobs from the config file
6feddc3
@KernAttila
feat: Convert custom commands options and pass them to the `DynamicSe…
03c08b6 
…ttingsWidget`
DiegoTavares
DiegoTavares previously requested changes Sep 25, 2024
View reviewed changes
Copy link
Collaborator

@DiegoTavares DiegoTavares left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

DiegoTavares pushed a commit that referenced this pull request Sep 25, 2024
@KernAttila
Cuebot reserve all cores (#1313)
da2fe01 
**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 <5556461+KernAttila@users.noreply.github.com>
@KernAttila
fix: for backward compatibility, check if we override cores by testin…
3c0a053 
…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.
@KernAttila
fix: revert to default behavior, we do not override only if the "core…
731bf80 
…s" value is None.
@KernAttila
fix: revert to default behavior, we do not override only if the "core…
f7062af 
…s" value is None.
@KernAttila
fix: only send "cores" to pyoutline, "overrideCores" is only set in c…
01d3fb2 
…uesubmit.
@KernAttila
fix: better UI scaling
b3408cc
@KernAttila
Merge branch 'master' into cuesubmit-jobs-from-config-file
34f2fea 
Signed-off-by: Kern Attila GERMAIN <5556461+KernAttila@users.noreply.github.com>
@KernAttila
fix: lint line too long
1bb0b7a
@KernAttila
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)
@KernAttila
Merge branch 'cuesubmit-jobs-from-config-file' of https://github.com/…
3b028b9 
…Wolf-Pipeline/OpenCue into cuesubmit-jobs-from-config-file
@KernAttila
fix: cast str to float
936f368
@KernAttila
feat: add opencue icon to cuesubmit window
73a66eb
@KernAttila
fix: add overrideCores attribute in LayerData.toDict()
134fd5d
@KernAttila
test: add overrideCores param
261db2f
@KernAttila
Merge branch 'master' into cuesubmit-jobs-from-config-file
fda4ab8
@KernAttila
Merge branch 'master' into cuesubmit-jobs-from-config-file
7e3b181
@KernAttila
Merge branch 'master' into cuesubmit-jobs-from-config-file
3b7fda4
@KernAttila KernAttila requested a review from lithorus as a code owner March 26, 2025 10:09
@DiegoTavares DiegoTavares self-requested a review March 26, 2025 21:31
@DiegoTavares DiegoTavares dismissed their stale review March 26, 2025 21:32

Assigning this review to Jimmy

@KernAttila KernAttila mentioned this pull request Apr 17, 2025
Open
@KernAttila
Copy link
Contributor Author

@lithorus and @DiegoTavares, this PR is up to date with the master branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lithorus lithorus lithorus left review comments

@bcipriano bcipriano Awaiting requested review from bcipriano bcipriano is a code owner

@gregdenton gregdenton Awaiting requested review from gregdenton

@jrray jrray Awaiting requested review from jrray

@smith1511 smith1511 Awaiting requested review from smith1511

@larsbijl larsbijl Awaiting requested review from larsbijl

@IdrisMiles IdrisMiles Awaiting requested review from IdrisMiles

@splhack splhack Awaiting requested review from splhack

@ramonfigueiredo ramonfigueiredo Awaiting requested review from ramonfigueiredo ramonfigueiredo is a code owner

@DiegoTavares DiegoTavares Awaiting requested review from DiegoTavares DiegoTavares is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@KernAttila @DiegoTavares @lithorus