Generalized functions for getting, setting, and finding environment variables

[elenya-grant]

Generalized functions for getting, setting, and finding environment variables

Ready for high-level review

Generalized the functions in h2integrate/resource/utilities/nlr_developer_api_keys.py for getting, finding, and setting environment variables. The functions in nlr_developer_api_keys.py were only usable for getting, finding, and setting environment variables for downloading resource data from NLR API calls.

The generalized functions now live in h2integrate/core/env_tools.py. The functions for NLR API calls have been updated to use these generalized functions.

This PR will make it easier for new environment variables to be introduced and used across models and tools without having to duplicate a bunch of code. Another benefit of this PR is that is has introduced more thorough testing for these methods.

A future PR could utilize this functionality to check if RESOURCE_DIR is specified as an environment variable.

If a developer is adding a new environment variable, they have to define 3 things:

1. global variable initialized as an empty string
2. setter/getter method for the global variable
3. wrapper of the get_environment_var function that includes the environment variable name (and an optional alternative/deprecated name).

For the NLR API email, this looks like:

# 1. global variable initialized as an empty string
developer_nlr_gov_key = ""

# 2. setter/getter method for the global variable
def set_developer_nlr_gov_key(var_value):
    global developer_nlr_gov_key
    if var_value is not None:
        developer_nlr_gov_key = var_value # setter
    return developer_nlr_gov_key # getter

from h2integrate.core.env_tools import get_environment_var
# 3. wrapper of the `get_environment_var` function that includes the environment variable name
get_nlr_developer_api_key = partial(
    get_environment_var,
    setter_method=set_developer_nlr_gov_key,
    varname_new="NLR_API_KEY", # new name
    varname_old="NREL_API_KEY", # optional, deprecated name
)

The environment variable can then be accessed/loaded from a script like below:

from h2integrate.resource.utilities.nlr_developer_api_keys import get_nlr_developer_api_key

api_key = get_nlr_developer_api_key()

Section 1: Type of Contribution

• Feature Enhancement
  • Framework
  • New Model
  • Updated Model
  • Tools/Utilities
  • Other (please describe):
• Bug Fix
• Documentation Update
• CI Changes
• Other (please describe):

Section 2: Draft PR Checklist

• Open draft PR
• Describe the feature that will be added
• Fill out TODO list steps
• Describe requested feedback from reviewers on draft PR
• [-] Complete Section 7: New Model Checklist (if applicable)

TODO:

• Add new feature (if reviewers want): check in "default" folders for a file that has a .env suffix (similar to how it checks possible default places for a .env file.
• Finish adding in tests for the functions in h2integrate/core/env_tools.py for "new" features if reviewers are interested
• Update docs (again, depending on whether new feature is integrated)

Type of Reviewer Feedback Requested (on Draft PR)

Structural feedback:

• Thoughts on the filename h2integrate/core/env_tools.py?
• General coding logic or areas for clean-up/improvement? (not for high-level review, but for in-depth review)
• Should the doc-strings for the functions in env_tools.py be updated to include documentation for all the input args or just the ones that are not input with the use of partial?

Implementation feedback:

• Do we want the added feature noted under the TODO section?
• Thoughts on the logic of the setter methods (such as set_developer_nlr_gov_email and set_developer_nlr_gov_key) which act as a "getter" if the var_value is None and act as a "setter" otherwise?
• Is there a case where an environment variable will not be a string or wouldn't be usable in string format? All the methods have logic checking if the length of the environment variable value is zero, but this logic would break if the environment variable is set to a float, integer, or boolean. I think that I'd prefer to keep the assumption that environment variables are strings because most of the loading or getting environment variable methods will return the environment variable as a string. (Depending on the responses to this, I may make a follow-on issue but will not integrate the ability for these methods to handle non-strings in this PR)

Section 3: General PR Checklist

• PR description thoroughly describes the new feature, bug fix, etc.
• Added tests for new functionality or bug fixes
• Tests pass (If not, and this is expected, please elaborate in the Section 6: Test Results)
• Documentation
  • Docstrings are up-to-date
  • Related docs/ files are up-to-date, or added when necessary
  • Documentation has been rebuilt successfully
  • [-] Examples have been updated (if applicable)
• CHANGELOG.md
  • At least one complete sentence has been provided to describe the changes made in this PR
  • After the above, a hyperlink has been provided to the PR using the following format:
    "A complete thought. [PR XYZ]((https://github.com/NatLabRockies/H2Integrate/pull/XYZ)", where
    XYZ should be replaced with the actual number.

Section 4: Related Issues

This is intended to resolve Issue #765

Issue #802 was created to note possible follow-on work that could be done once this PR is merged in!

Section 5: Impacted Areas of the Software

Section 5.1: New Files

• h2integrate/core/env_tools.py
  • _get_env_with_fallback: moved but function is unchanged except doc-string
  • load_file_with_variables: moved and made generalizable. Also updated to only be used with a single variable (rather than a list of variable names)
  • set_env_var_dot_env: generalized function of set_nlr_key_dot_env
  • get_env_var: generalized function of get_nlr_developer_api_key and get_nlr_developer_api_email

Section 5.2: Modified Files

• h2integrate/resource/utilities/nlr_developer_api_keys.py
  • get_nlr_developer_api_key: updated to leverage get_environment_var
  • get_nlr_developer_api_email: updated to leverage get_environment_var
  • set_nlr_email_key_dot_env: new function that leverages set_env_var_dot_env
  • set_nlr_api_key_dot_env: new function that leverages set_env_var_dot_env
  • set_nlr_key_dot_env: updated to use new functions set_nlr_email_key_dot_env and set_nlr_api_key_dot_env (Note that this function is not used in the main code but is used in conftest.py files)
  • removed _ENV_KEY_NEW, _ENV_KEY_OLD, etc. somewhat unnecessary at this point.

Section 6: Additional Supporting Information

Section 7: Test Results, if applicable

[RHammond2]

@elenya-grant I think this is a great step in the right direction, especially with moving much of the functionality out of the resource subpackage!

I have two significant hangups with the current draft: 1) we should generally avoid using global variables as a best practice, and 2) much of the refactored code is only lightly refactored, leaving it still highly geared towards an NLR credential getting/setting process. The second point results in this solution still being overly complex for general use.

I think that the core code could be something more like the following example that largely mirrors your setup (I've got a few mismatched names for expediency), but is more flexible to non-NLR credential getting and setting in addition to simplifying the NLR setup. I would also imagine using the module as from h2integrate.core.env_tools as api so we could invoke usage such as api.get_credentials("NLR_API_EMAIL", "NLR_API_KEY").

Another added benefit is that instead of a developer being required to do 3 steps to interact with API settings, the below reduces is it to simply calling api.set_env_var_from_file(".newapirc", "NEW_EMAIL", "NEW_KEY"). Later, if needed the credentials could be extracted as new_email = os.environ["NEW_EMAIL"] or new_email = api.get_credentials("NEW_EMAIL"). Alternatively, they could simply be retrieved as part of the module setup.

To be clear, I don't believe my code needs to be taken exactly as written as it's a prototype, but the straightforwardness
of the solution should be considered to resolve the heart of #765.

import os
import warnings
from pathlib import Path
from h2integrate import ROOT_DIR

HOME_DIR = Path.home()

NREL_DEPRECATION_MSG = (
    "The '{old}' environment variable is deprecated and will be removed in a future release. "
    "Please use '{new}' instead. The nrel.gov API domain has moved to nlr.gov."
)


def read_config(file_path: Path) -> dict:
    """Load any dictionary-like key, value pairs from a configuration file (e.g. .env or .cdsapirc)
    that uses either a ``key:value` or `key=value` format for storing data.

    Args:
        file_path (Path): The full file path and name containing configuration details to be
            extracted.

    Returns:
        dict: Dictionary of key, value pairs found in :py:attr:`file_path`.
    """
    if isinstance(file_path, str):
        file_path = Path(file_path).resolve()
    config = {}
    with file_path.open("r") as f:
        for line in f.readlines():
            if ":" in line:
                sep = ":"
            elif "=" in line:
                sep = "="
            k, v = line.strip().split(sep, 1)
            config[k.strip()] = v.strip()
    return config


def get_credentials(*args: str, file_name: str | None = None) -> dict:
    """Retrieve a series of credentials from a :py:attr:`file_name` in either the home directory
    or H2Integrate root directory.

    Args:
        args (str): Name(s) of the credential(s) that should be retrieved from either
            :py:attr:`file_name` or environment variables.
        file_name (str, optional): The name of a configuration file found in either the H2Integrate
            root directory or the user's home directory that should contain the credential(s) in
            :py:attr:`args`.

    Returns:
        dict: Dictionary of all :py:attr:`args` with values of either a found value or None.
    """
    if file_name is not None:
        if (file_path := (ROOT_DIR / file_name)).is_file():
            config = read_config(file_path)
        elif (file_path := (HOME_DIR / file_name)).is_file():
            config = read_config(file_path)
        return {name: config.get(name) for name in args}
    return {name: os.environ.get(name) for name in args}


def set_env_var_from_file(file_name: str, *args: str):
    """Sets a series of environment variables from a file base in the H2Integrate root directory
    or the user's home directory.

    Args:
        file_name (str): Name of the file found in either the H2Integrate root directory or the
            user's home directory that contains the variables to be set for the computational
            environment.
        args (str): Name(s) of the credentials to retrieve from :py:attr:`file_name`.

    Raises:
        KeyError: Raised if any of :py:attr:`args` wasn't found in :py:attr:`file_name`.
    """
    credentials = get_credentials(*args, file_name=file_name)
    for name, value in credentials.items():
        if value is None and os.environ.get(name) is None:
            raise KeyError(f"No credential for {name} was found in '{file_name}'")
        os.environ[name] = value


def get_nlr_api_credential(which: str) -> str:
    """Get either the NLR API email or key with a fallback for the NREL credentials.

    Args:
        which (str): One of "email" or "key" to indicate which NLR API credential should be
            retrieved.

    Raises:
        ValueError: Raised if an invalid value was passed to :py:attr:`which`.
        KeyError: Raised if neither of the NLR or NREL credentials could be found.
    """
    if which not in ("email", "key"):
        raise ValueError("`which` must be one of 'email' or 'key'.")
    new_name = f"NLR_API_{which.upper()}"
    old_name = f"NREL_API_{which.upper()}"
    credentials = get_credentials(new_name, old_name, file_name=".env")
    if (email := credentials[new_name]) is None:
        if (email := credentials[old_name]) is None:
            msg = (
                f"No credential matching {new_name} or {old_name} found as an environment variable"
                " nor the '.env' configuration file in the user home directory or H2Integrate root"
                " directory."
            )
            raise KeyError(msg)
        warnings.warn(
            NREL_DEPRECATION_MSG.format(old=old_name, new=new_name),
            FutureWarning,
            stacklevel=3,
        )
    return email


def set_nlr_api_credential(which: str):
    """Set an NLR API email or key with a fallback for the NREL credentials.

    Args:
        which (str): One of "email" or "key" to indicate which NLR API credential should be
            retrieved.

    Raises:
        ValueError: Raised if an invalid value was passed to :py:attr:`which`.
    """
    if which not in ("email", "key"):
        raise ValueError("`which` must be one of 'email' or 'key'.")
    os.environ[f"NLR_API_{which.upper()}"] = get_nlr_api_credential(which=which)

To answer one of your questions, I think the env_tools.py should be api_credentials.py or something with either "api" or "credentials" or similar in the name to indicate we're working specifically with API credentials. I think "env_tools.py" would be assumed to be coding environments more often than environment variables. The environment variables is also only one piece of the puzzle API credentials puzzle.

[elenya-grant]

Responding to this comment from @RHammond2:

First of all, thank you for the very helpful and thoughtful comment, this is very helpful in understanding your vision for resolving Issue #765. I like some of the added functionality that your approach introduces and I think I could get that into this PR (such as getting variables from a dictionary type formatted file). If I'm understanding your proposed code correctly, then I think that some of the existing functionality will be lost and other changes to the code may be needed (and some of this is related to the use of global variables). I'm also concerned about any lost functionality because it could mess with people's workflows. My understanding is that your proposed code would basically be used instead of the functions I have in env_tools.py - is that right?

I'm going to try to be concise and I'm sorry if it doesn't make sense! When I originally made the NLR API methods for the resource models, I was trying to make it so that finding/loading/setting/getting environment variables was as user-friendly as possible and would work for a variety of workflows and ways of setting environment variables while minimizing complexity within the resource models themselves.

Quick summary of the current (before this PR) functionality with the NLR API credentials and my understanding of it:

• If a user sets the environment variable using the preferred method then we wouldn't need to use global variables, this is easy because it doesnt require a read-file thing. As long as a user has done this type of set-up, the environment variable value is easily accessed with os.getenv("NLR_API_KEY") and the user does not have to run set_nlr_key_dot_env() at the start of each script. So when get_nlr_developer_api_key() is called within a resource model, it returns the environment variable without ever calling set_nlr_key_dot_env()
• If a user has the environment variables in a .env file thats in one of the possible locations (cwd/".env", ROOT_DIR/".env", or ROOT_DIR.parent/".env) and the environment variables haven't already been loaded/set (which is usually the case) then the set_nlr_key_dot_env() is called within get_nlr_developer_api_key() to first set the variable before returning it. This also wouldn't require the use of global variables.

The reason we're using global variables (and maybe we could use environment variables instead or something) is to accommodate for other user-workflow/preference cases. For example, a user is using a .env file to define the environment variables and that .env file is not in one of the "default" locations. If they were to try to run H2I with a resource model that needs the NLR_API_KEY, it would not be able to find the API key and the run would fail if their run script looked like this:

from h2integrate import H2IntegrateModel
h2i = H2IntegrateModel("config.yaml")
h2i.setup()
h2i.run()

Since the path to their ".env" file cannot be input to the resource models themselves, they need to instead call the setter method ahead of time so that the environment variables are set before the resource models call get_nlr_developer_api_key()

from h2integrate import H2IntegrateModel
from h2integrate.resource.utilities.nlr_developer_api_keys import set_nlr_key_dot_env
env_path = "Users/my/nonstandard/place/for/.env"
set_nlr_key_dot_env(path=env_path)
h2i = H2IntegrateModel("config.yaml")
h2i.setup()
h2i.run()

Basically, the get_api_key() style functions that are called in resource models are set-up so that a user doesnt have to call set_nlr_key_dot_env in every run-script if they do the preferred way of setting environment variables or they do it in the way thats instructed in the documentation (maybe except for the .yaml way - Idk how that works). But the set_nlr_key_dot_env is available so that folks can run resource models in H2I even if their workflow isn't one of the default supported ways of doing it.

Responses to other stuff:

I imagine that environment variables may be used to include more than just API credentials. Environment variables allow for folks to set a) info that shouldn't be shared and b) info that is specific to their workflow. For example, the RESOURCE_DIR can also be set as an environment variable and that'd be used instead of DEFAULT_RESOURCE_DIR (Issue #802). This is why I named things with env instead of api.

Related to above, I think that the read_config method may have issues if someone is using a Windows folder path as the environment variable value (RESOURCE_DIR = C:\\Users\whatever) because it'd split the filepath at :.

Perhaps some of my push-back is because I'm misinterpreting your comment "it still highly geared towards an NLR credential getting/setting process". Do you mean specific to the NLR_API_KEY and NLR_API_EMAIL variables used for resource data downloads OR do you mean like some workflow that is common at NLR that isn't common elsewhere? I think the only NLR-specific part of my proposed changes is the depreciation method warning.

Let me know if you want to hop on a call and chat about through some of this! I would be down to integrate a form of the read_config method you shared! Thanks again for the thoughtful comment!