Skip to content

Csub silently ignores extra env#28

Open
alexdremov wants to merge 7 commits intoepfml:mainfrom
alexdremov:patch-5
Open

Csub silently ignores extra env#28
alexdremov wants to merge 7 commits intoepfml:mainfrom
alexdremov:patch-5

Conversation

@alexdremov
Copy link
Copy Markdown
Contributor

Before, csub was just ignoring extra env from .env file, which is unexpected and leads to bugs

Copilot AI review requested due to automatic review settings March 23, 2026 16:43
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to stop csub from silently ignoring additional .env variables by ensuring the container runtime environment includes the full .env contents (not just a fixed subset).

Changes:

  • Build literal_env by merging the full parsed .env dict with runtime overrides (HOME, NB_UID, etc.).
  • Adjust how environment flags are added to the generated runai submit command.
Comments suppressed due to low confidence (1)

csub.py:120

  • literal_env is now built as env | {...}, which pulls in all keys from the .env file, including secret values (e.g., WANDB_API_KEY, HF_TOKEN, GITHUB_TOKEN, etc.). If these are passed through --environment KEY=value, the secret values will end up in the generated command line / logs instead of being sourced from the Kubernetes secret. Consider filtering secret keys out of the dict passed to add_env_flags, and continue injecting secrets via add_secret_env_flags (using EXTRA_SECRET_KEYS to extend the secret key set).
    literal_env = env | {
        "HOME": f"/home/{env['LDAP_USERNAME']}",
        "NB_USER": env["LDAP_USERNAME"],
        "NB_UID": run_uid,
        "NB_GROUP": env["LDAP_GROUPNAME"],
        "NB_GID": run_gid,
        "WORKING_DIR": working_dir,
        "SCRATCH_HOME": scratch_home,
        "SCRATCH_HOME_ROOT": scratch_root,
        "EPFML_LDAP": env["LDAP_USERNAME"],
        "HF_HOME": hf_home,
        "UV_PYTHON_VERSION": env.get("UV_PYTHON_VERSION", "3.11"),
        "TZ": env.get("TZ", "Europe/Zurich"),
        # Keep runtime shell and tool caches available when using `runai exec`
        "GIT_CONFIG_GLOBAL": f"{scratch_home}/.gitconfig",
        "UV_CACHE_DIR": f"{scratch_home}/.cache/uv",
        "UV_PYTHON_INSTALL_DIR": f"{scratch_home}/.uv",
    }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

alexdremov and others added 2 commits April 9, 2026 16:36
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.

2 participants