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

[Serve] Support env var in service yaml #3078

Merged
merged 6 commits into from
Feb 8, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 16 additions & 8 deletions sky/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,18 +72,22 @@ def _is_valid_name(name: Optional[str]) -> bool:
return bool(re.fullmatch(_VALID_NAME_REGEX, name))


def _fill_in_env_vars_in_file_mounts(
file_mounts: Dict[str, Any],
def _fill_in_env_vars(
yaml_field: Dict[str, Any],
task_envs: Dict[str, str],
) -> Dict[str, Any]:
"""Detects env vars in file_mounts and fills them with task_envs.
"""Detects env vars in yaml field and fills them with task_envs.

Use cases of env vars in file_mounts:
- dst/src paths; e.g.,
/model_path/llama-${SIZE}b: s3://llama-weights/llama-${SIZE}b
- storage's name (bucket name)
- storage's source (local path)

Use cases of env vars in service:
- model type; e.g.,
model: $MODEL_NAME
dtran24 marked this conversation as resolved.
Show resolved Hide resolved

We simply dump file_mounts into a json string, and replace env vars using
dtran24 marked this conversation as resolved.
Show resolved Hide resolved
regex. This should be safe as file_mounts has been schema-validated.
dtran24 marked this conversation as resolved.
Show resolved Hide resolved

Expand All @@ -93,7 +97,7 @@ def _fill_in_env_vars_in_file_mounts(
where <ENV> must appear in task.envs.
"""
# TODO(zongheng): support ${ENV:-default}?
file_mounts_str = json.dumps(file_mounts)
yaml_field_str = json.dumps(yaml_field)

def replace_var(match):
var_name = match.group(1)
Expand All @@ -102,8 +106,8 @@ def replace_var(match):

# Pattern for valid env var names in bash.
pattern = r'\$\{?\b([a-zA-Z_][a-zA-Z0-9_]*)\b\}?'
file_mounts_str = re.sub(pattern, replace_var, file_mounts_str)
return json.loads(file_mounts_str)
yaml_field_str = re.sub(pattern, replace_var, yaml_field_str)
return json.loads(yaml_field_str)


def _check_docker_login_config(task_envs: Dict[str, str]) -> bool:
Expand Down Expand Up @@ -366,8 +370,12 @@ def from_yaml_config(
# Fill in any Task.envs into file_mounts (src/dst paths, storage
# name/source).
if config.get('file_mounts') is not None:
config['file_mounts'] = _fill_in_env_vars_in_file_mounts(
config['file_mounts'], config.get('envs', {}))
config['file_mounts'] = _fill_in_env_vars(config['file_mounts'],
config.get('envs', {}))

dtran24 marked this conversation as resolved.
Show resolved Hide resolved
if config.get('service') is not None:
config['service'] = _fill_in_env_vars(config['service'],
config.get('envs', {}))

task = Task(
config.pop('name', None),
Expand Down
Loading