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.

Sign up for GitHub

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

FLYTECTL_CONFIG env var should take highest precedence #1662

Merged
merged 3 commits into from
May 31, 2023
Merged

FLYTECTL_CONFIG env var should take highest precedence #1662

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f222cec
move env var up when looking for config file
wild-endeavor May 29, 2023
57da075
pr comment
wild-endeavor May 30, 2023
c8ec8bb
comment
wild-endeavor May 30, 2023
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
8 changes: 8 additions & 0 deletions flytekit/clis/sdk_in_container/pyflyte.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import os
import typing

import grpc
Expand All @@ -17,6 +18,7 @@
from flytekit.clis.sdk_in_container.run import run
from flytekit.clis.sdk_in_container.serialize import serialize
from flytekit.clis.sdk_in_container.serve import serve
from flytekit.configuration.file import FLYTECTL_CONFIG_ENV_VAR, FLYTECTL_CONFIG_ENV_VAR_OVERRIDE
from flytekit.configuration.internal import LocalSDK
from flytekit.exceptions.base import FlyteException
from flytekit.exceptions.user import FlyteInvalidInputException
Expand Down Expand Up @@ -120,6 +122,12 @@ def main(ctx, pkgs: typing.List[str], config: str, verbose: bool):
if config:
ctx.obj[CTX_CONFIG_FILE] = config
cfg = configuration.ConfigFile(config)
# Set here so that if someone has Config.auto() in their user code, the config here will get used.
if FLYTECTL_CONFIG_ENV_VAR in os.environ:
cli_logger.info(
f"Config file arg {config} will override env var {FLYTECTL_CONFIG_ENV_VAR}: {os.environ[FLYTECTL_CONFIG_ENV_VAR]}"
)
os.environ[FLYTECTL_CONFIG_ENV_VAR_OVERRIDE] = config
if not pkgs:
pkgs = LocalSDK.WORKFLOW_PACKAGES.read(cfg)
if pkgs is None:
Expand Down
22 changes: 16 additions & 6 deletions flytekit/configuration/file.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@

# This is the env var that the flytectl sandbox instructions say to set
FLYTECTL_CONFIG_ENV_VAR = "FLYTECTL_CONFIG"
# This is an explicit override only to be used by pyflyte and takes precedence in get_config_file over the main
# environment variable.
# This env var should not be set by users
FLYTECTL_CONFIG_ENV_VAR_OVERRIDE = "_FLYTECTL_CONFIG_PYFLYTE_OVERRIDE"


def _exists(val: typing.Any) -> bool:
Expand Down Expand Up @@ -239,6 +243,17 @@ def get_config_file(c: typing.Union[str, ConfigFile, None]) -> typing.Optional[C
Checks if the given argument is a file or a configFile and returns a loaded configFile else returns None
"""
if c is None:
# Pyflyte override env var takes highest precedence
# Env var takes second highest precedence
flytectl_path_from_env = getenv(FLYTECTL_CONFIG_ENV_VAR_OVERRIDE, getenv(FLYTECTL_CONFIG_ENV_VAR, None))
if flytectl_path_from_env:
flytectl_path = Path(flytectl_path_from_env)
if flytectl_path.exists():
logger.info(f"Using flytectl/YAML config {flytectl_path.absolute()}")
return ConfigFile(str(flytectl_path.absolute()))
else:
logger.warning(f"flytectl config file {flytectl_path.absolute()} does not exist, ignoring...")

# See if there's a config file in the current directory where Python is being run from
current_location_config = Path("flytekit.config")
if current_location_config.exists():
Expand All @@ -251,13 +266,8 @@ def get_config_file(c: typing.Union[str, ConfigFile, None]) -> typing.Optional[C
logger.info(f"Using configuration from home directory {home_dir_config.absolute()}")
return ConfigFile(str(home_dir_config.absolute()))

# If not, see if the env var that flytectl sandbox tells the user to set is set,
# or see if there's something in the default home directory location
# If not see if there's something in the default home directory location
flytectl_path = Path(Path.home(), ".flyte", "config.yaml")
flytectl_path_from_env = getenv(FLYTECTL_CONFIG_ENV_VAR, None)

if flytectl_path_from_env:
flytectl_path = Path(flytectl_path_from_env)
if flytectl_path.exists():
logger.info(f"Using flytectl/YAML config {flytectl_path.absolute()}")
return ConfigFile(str(flytectl_path.absolute()))
Expand Down