Skip to content

Commit

Permalink
Fix value resolution in resolve_env_vars() (microsoft#195)
Browse files Browse the repository at this point in the history
- Removed uppercase conversion for envvar values in
`resolve_env_vars()`. Now envvar name is converted to uppercase, but
value is left as-is.
- Added logging so users can understand why env resolution fails.
- Refactored a bit to reduce indentation levels so that logging messages
fit with current line length limitation.
- Edited docs to be consistent with uppercase envvar names.
- Fixed typo in workflow name (said "ci" when it was "pr").
  • Loading branch information
mariamedp authored Sep 4, 2024
1 parent 9dcdfe4 commit a22290d
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 26 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/class_flows_pr_dev_workflow.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
name: class_flows_ci_dev_workflow
name: class_flows_pr_dev_workflow

on:
workflow_call:
Expand Down
2 changes: 1 addition & 1 deletion docs/faqs.md
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ APPLICATIONINSIGHTS_CONNECTION_STRING=xxxx

### What values should go into .env file and ENV_VARS ?

If you have configurations in flows (init.json, flow.flex.yaml), experiment.yaml and env.yaml files that uses ${secret_name} style placeholders, then the actual value for secret_name should be stored in ENV_VARS secret in github, library variable in Azure DevOps and within .env file for local execution. These placeholders will be replaced by the actual values during execution either from ENV_VARS secret in github, library variable in Azure DevOps or from .env file for local execution.
If you have configurations in flows (init.json, flow.flex.yaml), experiment.yaml and env.yaml files that uses ${SECRET_NAME} style placeholders, then the actual value for SECRET_NAME should be stored in ENV_VARS secret in github, library variable in Azure DevOps and within .env file for local execution. These placeholders will be replaced by the actual values during execution either from ENV_VARS secret in github, library variable in Azure DevOps or from .env file for local execution.

- `experiment.yaml` file should contain placeholders for api_key within connections element.
- `env.yaml` file should contain placeholders for all other configurations. Check use case `function_flows` for an example. It has multiple ${} placeholder within env.yaml.
Expand Down
47 changes: 27 additions & 20 deletions llmops/common/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def get_property_value(self):
raise ValueError("Neither 'pf' nor 'ml_client' is available")


def resolve_env_vars(base_path: str) -> Dict:
def resolve_env_vars(base_path: str, logger: logging.Logger) -> Dict:
"""
Resolve the environment variables from the config files.
Expand All @@ -55,33 +55,40 @@ def resolve_env_vars(base_path: str) -> Dict:
env_vars = {}
yaml_file_path = os.path.join(base_path, "environment", "env.yaml")
if os.path.isfile(os.path.abspath(yaml_file_path)):
logger.info(f"Reading envvars from '{yaml_file_path}'")
with open(yaml_file_path, "r") as file:
yaml_data = yaml.safe_load(file)
for key, value in yaml_data.items():
key = str(key).strip().upper()
value = str(value).strip().upper()

temp_val = os.environ.get(key, None)
if temp_val is not None:
env_vars[key] = os.environ.get(key, None)
else:
if (
isinstance(value, str)
and value.startswith('${')
and value.endswith('}')
):
value = value.replace('${', '').replace('}', '')
resolved_value = os.environ.get(value, None)
value = str(value).strip()

value_existing = os.environ.get(key, None)
if value_existing is not None:
logger.info(f"Envvar {key} defined in env, using that value.")
env_vars[key] = value_existing
elif value is None or len(value) == 0:
raise ValueError(f"{key} in env.yaml not resolved")
elif (
isinstance(value, str)
and value.startswith('${')
and value.endswith('}')
):
value_reference = value.replace('${', '').replace('}', '')
try:
resolved_value = os.environ[value_reference]
logger.info(f"Reference {value} resolved.")
os.environ[key] = str(resolved_value)
env_vars[key] = str(resolved_value)
elif value is None or len(value) == 0:
raise ValueError(f"{key} in env.yaml not resolved")
else:
os.environ[key] = str(value)
env_vars[key] = str(value)
except KeyError as e:
raise ValueError(
f"Reference {value} could not be resolved") from e
else:
os.environ[key] = str(value)
env_vars[key] = str(value)
logger.info(f"Envvar {key} loaded.")
else:
env_vars = {}
print("no values")
logger.info("No env file found.")
return env_vars


Expand Down
2 changes: 1 addition & 1 deletion llmops/common/deployment/kubernetes_deployment.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ def create_kubernetes_deployment(
key, value = key_value
params_dict[key] = value

env_vars = resolve_env_vars(experiment.base_path)
env_vars = resolve_env_vars(experiment.base_path, logger)

real_config = f"{base_path}/configs/deployment_config.json"

Expand Down
2 changes: 1 addition & 1 deletion llmops/common/deployment/provision_deployment.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ def create_deployment(
key, value = key_value
params_dict[key] = value

env_vars = resolve_env_vars(experiment.base_path)
env_vars = resolve_env_vars(experiment.base_path, logger)

real_config = f"{base_path}/configs/deployment_config.json"

Expand Down
2 changes: 1 addition & 1 deletion llmops/common/prompt_eval.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ def prepare_and_execute(

flow_type, params_dict = resolve_flow_type(evaluator.path, "")

env_vars = resolve_env_vars(experiment.base_path)
env_vars = resolve_env_vars(experiment.base_path, logger)

dataframes = []
metrics = []
Expand Down
3 changes: 2 additions & 1 deletion llmops/common/prompt_pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ def prepare_and_execute(
ml_client = None
wrapper = None
if EXECUTION_TYPE == "LOCAL":
logger.info("Creating promptflow connections for local execution.")
pf = PFClientLocal()
create_pf_connections(
exp_filename,
Expand Down Expand Up @@ -207,7 +208,7 @@ def prepare_and_execute(
all_metrics = []

env_vars = {}
env_vars = resolve_env_vars(experiment.base_path)
env_vars = resolve_env_vars(experiment.base_path, logger)

logger.info(f"Running experiment {experiment.name}")
for mapped_dataset in experiment.datasets:
Expand Down

0 comments on commit a22290d

Please sign in to comment.