Skip to content

Expose gigl vars as env vars for the custom luancher#642

Merged
kmontemayor2-sc merged 6 commits into
mainfrom
kmonte/env-vars-for-custom-launcher
May 20, 2026
Merged

Expose gigl vars as env vars for the custom luancher#642
kmontemayor2-sc merged 6 commits into
mainfrom
kmonte/env-vars-for-custom-launcher

Conversation

@kmontemayor2-sc
Copy link
Copy Markdown
Collaborator

Do this so custom launchers can get the task config, etc.

The alternative approach here is that we can use omegaconf but as we discussed offline that may be more complicated as the resolution would change often.

Comment thread gigl/src/common/custom_launcher.py
Copy link
Copy Markdown
Collaborator

@svij-sc svij-sc left a comment

Choose a reason for hiding this comment

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

Might be good opportunity to also consolidate existing VAI launchers to use env vars too instead of injected runtime args:

job_args = (
[
f"--job_name={job_name}",
f"--task_config_uri={task_config_uri}",
f"--resource_config_uri={resource_config_uri}",
]
+ (["--use_cuda"] if use_cuda else [])
+ ([f"--{k}={v}" for k, v in args.items()])
)

Adopting one distinct pattern will make it easier for us to maintain the different code paths, and I am happy to adapt the env variables given prior discussed arguments as the golden path, but lets also old our old injection paths in existing launchers.

Comment thread gigl/env/constants.py
Comment thread gigl/env/constants.py
Comment thread gigl/env/constants.py Outdated
Comment thread tests/unit/src/common/custom_launcher_test.py Outdated
Comment thread tests/unit/src/common/custom_launcher_test.py
Copy link
Copy Markdown
Collaborator

@svij-sc svij-sc left a comment

Choose a reason for hiding this comment

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

I'd prefer to have this PR in first as the other changes may be somewhat complicated still (and I'm not sure we need all of these things to be exposed globally)

Approving conditionally - only for the purposes of unblocking. I am hoping we do a fast follow up here to address concerns outlined below:

  1. https://github.com/Snapchat/GiGL/pull/642/changes#r3268620564
  2. https://github.com/Snapchat/GiGL/pull/642/changes#r3269575120

@kmontemayor2-sc kmontemayor2-sc marked this pull request as ready for review May 20, 2026 17:21
@kmontemayor2-sc kmontemayor2-sc added this pull request to the merge queue May 20, 2026
Merged via the queue into main with commit 02d64a9 May 20, 2026
1 check passed
@kmontemayor2-sc kmontemayor2-sc deleted the kmonte/env-vars-for-custom-launcher branch May 20, 2026 18:57
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

Successfully merging this pull request may close these issues.

4 participants