Skip to content
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

Better handling of environments for farm jobs #876

Open
2 tasks done
antirotor opened this issue Sep 9, 2024 · 5 comments
Open
2 tasks done

Better handling of environments for farm jobs #876

antirotor opened this issue Sep 9, 2024 · 5 comments

Comments

@antirotor
Copy link
Member

Is there an existing issue for this?

  • I have searched the existing issues.

Please describe the feature you have in mind and explain what the current shortcomings are?

Currently, every integrator in Deadline (or for farm as it is) that is creating jobs is setting (or passing) environments to the job independently. Various submit_*_deadline handle this logic by themselves. Also, they are assuming existence of addon specific environments. This is creating hidden dependency between addons and it makes adding new environments difficult.

How would you imagine the implementation of the feature?

This should be job of collectors in individual addons. For example collector in ayon-core should collect context and instance level environments like:

"AYON_DEFAULT_SETTINGS_VARIANT"  # context
"AYON_BUNDLE_NAME"  # context
"AYON_PROJECT_NAME"  # context
"AYON_USERNAME"  # context
"IS_TEST"  # context

"AYON_FOLDER_PATH"  # instance
"AYON_TASK_NAME"  # instance
"AYON_WORKDIR"  # instance

In ftrack addon:

"FTRACK_API_KEY"  # context
"FTRACK_API_USER"  # context
"FTRACK_SERVER"  # context

etc.

Also, because not every type of job needs all env vars, those collectors should be aware for what type of jobs they are providing what environment variable. This admittedly creates soft dependency too - at least between addons and ayon-core that should define job types, but that is reasonable.

To achieve this, ayon-core should provide API used by those collectors and farm integrators - ability to get/set environments for different job types and provide fallback option for unknow (uset) job type.

Are there any labels you wish to add?

  • I have added the relevant labels to the enhancement request.

Describe alternatives you've considered:

Backwards compatibility should be solved by keeping the logic in farm integrators but executing it only if collected environments are not provided.

Additional context:

No response

@antirotor antirotor added the type: enhancement Improvement of existing functionality or minor addition label Sep 9, 2024
@antirotor antirotor changed the title Better handling of environments for job farm Better handling of environments for farm jobs Sep 9, 2024
@BigRoy
Copy link
Collaborator

BigRoy commented Sep 9, 2024

I'm not entirely sure about that "soft dependency". I think currently, there's hardly any soft depedency currently?

Preferably we just have a job_environments dict that we can maintain for context and instances that get passed onto the deadline submissions dynamically without every host-specific submitter needing to do that manually (avoiding code duplication).

class DeadlineBaseSubmitter:
    def get_submission_env(self, instance) -> dict:
        env = instance.context.get("job_environments", {})
        env.update(instance.get("job_environments", {}))
        return env

    def submit(self, instance, payload):
        env = self.get_submission_env(instance)
        payload["env"] = env # or however we populate the env vars at this stage
        requests.post(deadline, payload)

etc.

  • Then any submission plug-in can manually override get_submission_env if specifics are needed there.
  • We can have a global collector that does the usual defaults like task name, etc.
  • We can have addon or studio specific collectors appending specific job vars if needed without tweaking the submitters.
  • We can reuse the same context and instance data for e.g. other farm integrations, like royal render.

I'm still not sure how the "soft dependency" is really needed. It'd be good to write out the different use cases of env vars we actually want to set and how they differ in certain situations.

@antirotor
Copy link
Member Author

Soft dependency isn't needed, quite opposite - we need to get rid of it. And I agree, these steps are definitely what needs to be done. Deadline submitter shoudln't be aware of any ftrack, kitsu and similar environment variables - ftrack, kitsu and similar shouldn't be aware of deadline. They need to be aware of farm job types so they can decide what variables are needed.

The soft dependency I was mentioning is currently in the fact, that almost every submitter plugin in Deadline is adding addon specific environments - so if we add another addon that we need it's environments to be passed to Deadline, we need to modify all deadline submitter plugins.

Also, it needs to be farm manager agnostic (there is also RoyalRender). Maybe @iLLiCiTiT has something to add here too :)

I imagine something like JobEnvironments() class defined in ayon-core that adds method to set/add/get environments for specific job type (along with job type enumerator). Also collector running on context using this class to add context specific environments. Then add similar addon to every possible addon (ftrack, kitsu, etc.) that is using the same class and adding data to instance. Then changing the submitters with the similar code you've proposed plus using the environment collected by the collectors.

@iLLiCiTiT
Copy link
Member

iLLiCiTiT commented Sep 11, 2024

Right now there are 2 types of farm jobs, render and publish. Render should not need ftrack or kitsu login (or does it?). At the same time we might need to add more job types in future (e.g. remote publish) that might have different requirements.

From my point of view, the most important is that we're able to set them our of farm addon plugins. Addon has control over what is added per instance or globally on context.

It was me who's idea was to be more granular, but if we would add all environments to all jobs it is probably not a big deal. But do we want to? Can there be a usecase where setting env in render or publish job would break the other one? @antirotor I might need help here, because I don't remember if there was a reason, or we were just thinking too much?

@BigRoy
Copy link
Collaborator

BigRoy commented Oct 8, 2024

Currently with ynput/ayon-deadline#32 implemented we have now de-duplicated the code and use a JOB_ENV_DATA_KEY to store the relevant environment variables (for render jobs only) on the instance and context data - however this does not allow us to differentiate which keys are relevant to which type of job.

So one of the next steps would be to now redesign this to support defining what job we'd target.
However, I'd argue that if going that direction next we're better off making a bigger redesign that allows us to set up jobs, dependent on each other easily so that e.g. "tile assembly" wouldn't need to be embedded within the plug-in that is doing the render submission.

We may need to form something of a 'flow' where one could make job submissions dependent on each other or make it absolutely trivial to add a job, with submissions, and env vars specific to it.

For example, say we have instance.data["farmJobs"] which could be a Dict[str, Any] of e.g.:

from ayon_core.pipeline import FarmJob

farm_jobs = {
    "render": FarmJob(name="render", payload=render_payload),
    "export": FarmJob(name="export", payload=export_payload, depends_on={"render",})
}

Then we could just have a single SubmitJobs plug-in that would handle the Deadline submission of ALL the jobs. It would then submit them in order of their dependencies and set that up for you, etc.

We'll need to abstract it away somewhere.
Any API proposals to do so?

Once we have 'named' jobs then it might make it easy to also target env vars to the named jobs, etc.

@BigRoy
Copy link
Collaborator

BigRoy commented Nov 5, 2024

A lot of this is being resolved by ynput/ayon-deadline#49 - if not all.
It may be worth to investigate after what would be the next goals - and potentially redefine the issue to target only the remainder open things instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants