-
Notifications
You must be signed in to change notification settings - Fork 12
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.
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
Implement slapcomp workflow #95
base: master
Are you sure you want to change the base?
Conversation
Saw an issues with retrieving the |
@@ -135,4 +138,10 @@ def parse_options(self, render_globals): | |||
state = "Suspended" if attributes["suspendPublishJob"] else "Active" | |||
options["suspendPublishJob"] = state | |||
|
|||
# Check if the run slap comp | |||
if attributes["runSlapComp"]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will break backwards compatibility for render global nodes that don't have the slap comp attribute yet, right?
comment = instance.context.data["comment"] | ||
|
||
scriptdir = _get_script_dir() | ||
scriptfile = os.path.join(scriptdir, "deadline_swith_and_submit.py") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you test this? :) Probably won't work because of a typo. The filename is: deadline_switch_and_submit.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Saw an issues with retrieving the listedslaves, will fix this before merging
I saw this as well when attempting to fix the above mentioned issue
self.log.info("Slap comp arguments: %s" % args) | ||
|
||
def _get_list_type(self, job): | ||
return "Whitelist" if job["Props"]["White"]is True else "Blacklist" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing space in job["Props"]["White"]is
- it's interesting that this is actually valid python code.
@@ -159,3 +159,7 @@ def process(self, instance): | |||
response = requests.post(url, json=payload) | |||
if not response.ok: | |||
raise Exception(response.text) | |||
|
|||
# Temporary key name, deadlineSubmissionJob was already taken | |||
if instance.data("runSlapComp", False): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is doing what you intended. Probably meant: instance.data.get("runSlapComp", False)
?
e7f6c56 Didn't you merge the conflicts the wrong way? If this branch is supposed to start using |
import os | ||
|
||
|
||
site.addsitedir(r"P:\pipeline\dev\git\env_prototype") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove :)
# by temporarily taking on its `PATH` settings | ||
original = os.environ["PATH"] | ||
os.environ["PATH"] = env.get("PATH", os.environ.get("PATH", "")) | ||
exe = lib.which("fusionconsolenode") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could use the acre.which
? So you don't need to use the hack?
colorbleed/scripts/launch.py
Outdated
|
||
|
||
config_root = os.path.dirname(os.path.dirname(__file__)) | ||
os.environ["TOOL_ENV"] = os.path.join(config_root, "environments") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not correct? Since we have a standalone repo for the environments.
colorbleed/scripts/launch.py
Outdated
raise ValueError("Unable to find executable: %s" % executable) | ||
|
||
print("Launching: %s" % exe) | ||
acre.launch(exe, environment=env, args=args, cwd=env.get("AVALON_WORKDIR")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wasn't this full file's functionality replaced by acre's launching?
Had a quick look - some comments. Test afterwards, and just get this beast merged. But ensure it's not going to break production please 👍 Also ensure you have everything in which might break backwards compatibility later - otherwise it'll bite you. |
I want this. Still troublesome, eh? :) |
Main points
-Note: part of issue FUS-32-