Skip to content

Commit

Permalink
fix: deepspeed e2e_tests fail when environment variable contains a ne…
Browse files Browse the repository at this point in the history
…wline character [FE-256] (#8154)

* fix: deepspeed e2e_tests fail when environment variable contains a newline character [FE-256]

* Fixed lint formatting issue

* Fixed a line too long issue

* Changed 'logging.debug()' to 'logging.warning()'.  The use of 'loggin.warn()' is deprecated.
  • Loading branch information
rcorujo authored Oct 25, 2023
1 parent ec7e004 commit f9c6402
Showing 1 changed file with 23 additions and 2 deletions.
25 changes: 23 additions & 2 deletions harness/determined/launch/deepspeed.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,30 @@ def create_deepspeed_env_file() -> None:
# since values may contain spaces and quotes. shlex.quote was removed from the
# deepspeed launcher in 0.6.2 so we add it here for this version onwards.
if deepspeed_version >= version.parse("0.6.2"):
f.write(f"{k}={shlex.quote(v)}\n")
line = f"{k}={shlex.quote(v)}"
else:
f.write(f"{k}={v}\n")
line = f"{k}={v}"

# By default, IFS is set to a space, a tab, and a newline character.
# Therefore, the IFS environment variable will be written as:
#
# IFS='
# '
#
# The single quote on its own line will cause the
# "key, val = var.split('=', maxsplit=1)" in
# "deepspeed/launcher/runner.py" to fail with the error:
#
# ValueError: not enough values to unpack (expected 2, got 1)
#
# Therefore, avoid writing any environment variables containing a
# newline character.
if "\n" not in line:
f.write(f"{line}\n")
else:
logging.warning(
f"Excluding environment variable {k} because it contains a newline character."
)


def create_run_command(master_address: str, hostfile_path: Optional[str]) -> List[str]:
Expand Down

0 comments on commit f9c6402

Please sign in to comment.