From 7f79d9c3899cc8e6b688f6d56c9f799514a8b41d Mon Sep 17 00:00:00 2001 From: Danny Chiao Date: Wed, 15 Sep 2021 13:48:57 -0400 Subject: [PATCH 01/10] Add flags file to include experimental flags and test/usage flags Signed-off-by: Danny Chiao --- Makefile | 2 +- sdk/python/feast/cli.py | 11 +++++- sdk/python/feast/errors.py | 9 +++++ sdk/python/feast/feature_store.py | 9 ++++- sdk/python/feast/flags.py | 39 +++++++++++++++++++ .../feast/infra/offline_stores/bigquery.py | 5 +-- sdk/python/feast/usage.py | 5 +-- sdk/python/usage_tests/test_usage.py | 11 +++--- 8 files changed, 75 insertions(+), 16 deletions(-) create mode 100644 sdk/python/feast/flags.py diff --git a/Makefile b/Makefile index f7fa7cdbee..daf6e8bad0 100644 --- a/Makefile +++ b/Makefile @@ -53,7 +53,7 @@ install-python: python -m pip install -e sdk/python -U --use-deprecated=legacy-resolver test-python: - FEAST_USAGE=False pytest -n 8 sdk/python/tests + FEAST_USAGE=False IS_TEST=True pytest -n 8 sdk/python/tests test-python-integration: FEAST_USAGE=False IS_TEST=True pytest -n 8 --integration sdk/python/tests diff --git a/sdk/python/feast/cli.py b/sdk/python/feast/cli.py index c7d961c2f0..14d60864bd 100644 --- a/sdk/python/feast/cli.py +++ b/sdk/python/feast/cli.py @@ -21,8 +21,12 @@ import pkg_resources import yaml -from feast import utils -from feast.errors import FeastObjectNotFoundException, FeastProviderLoginError +from feast import flags, utils +from feast.errors import ( + ExperimentalFeatureNotEnabled, + FeastObjectNotFoundException, + FeastProviderLoginError, +) from feast.feature_store import FeatureStore from feast.repo_config import load_repo_config from feast.repo_operations import ( @@ -416,6 +420,9 @@ def init_command(project_directory, minimal: bool, template: str): @click.pass_context def serve_command(ctx: click.Context, port: int): """[Experimental] Start a the feature consumption server locally on a given port.""" + if not flags.enable_python_feature_server(): + raise ExperimentalFeatureNotEnabled(flags.FLAG_PYTHON_FEATURE_SERVER_NAME) + repo = ctx.obj["CHDIR"] cli_check_repo(repo) store = FeatureStore(repo_path=str(repo)) diff --git a/sdk/python/feast/errors.py b/sdk/python/feast/errors.py index cf85bdd08c..6ab6d82edd 100644 --- a/sdk/python/feast/errors.py +++ b/sdk/python/feast/errors.py @@ -2,6 +2,8 @@ from colorama import Fore, Style +from feast.flags import FLAG_ALPHA_FEATURES_NAME + class DataSourceNotFoundException(Exception): def __init__(self, path): @@ -258,3 +260,10 @@ def __init__(self, feature_view_name: str): super().__init__( f"The feature view name: {feature_view_name} refers to both an on-demand feature view and a feature view" ) + + +class ExperimentalFeatureNotEnabled(Exception): + def __init__(self, feature_flag_name: str): + super().__init__( + f"Trying to use experimental feature. Please ensure to set environment variables {FLAG_ALPHA_FEATURES_NAME} and {feature_flag_name} to true" + ) diff --git a/sdk/python/feast/feature_store.py b/sdk/python/feast/feature_store.py index 586ff2f0bc..471fb85b97 100644 --- a/sdk/python/feast/feature_store.py +++ b/sdk/python/feast/feature_store.py @@ -22,11 +22,12 @@ from colorama import Fore, Style from tqdm import tqdm -from feast import feature_server, utils +from feast import feature_server, flags, utils from feast.data_source import RequestDataSource from feast.entity import Entity from feast.errors import ( EntityNotFoundException, + ExperimentalFeatureNotEnabled, FeatureNameCollisionError, FeatureViewNotFoundException, RequestDataNotFoundInEntityDfException, @@ -380,6 +381,9 @@ def apply( views_to_update = [ob for ob in objects if isinstance(ob, FeatureView)] odfvs_to_update = [ob for ob in objects if isinstance(ob, OnDemandFeatureView)] + if not flags.enable_on_demand_feature_views() and len(odfvs_to_update) > 0: + raise ExperimentalFeatureNotEnabled(flags.FLAG_ON_DEMAND_TRANSFORM_NAME) + _validate_feature_views(views_to_update) entities_to_update = [ob for ob in objects if isinstance(ob, Entity)] services_to_update = [ob for ob in objects if isinstance(ob, FeatureService)] @@ -986,6 +990,9 @@ def _augment_response_with_on_demand_transforms( @log_exceptions_and_usage def serve(self, port: int) -> None: """Start the feature consumption server locally on a given port.""" + if not flags.enable_python_feature_server(): + raise ExperimentalFeatureNotEnabled(flags.FLAG_PYTHON_FEATURE_SERVER_NAME) + feature_server.start_server(self, port) diff --git a/sdk/python/feast/flags.py b/sdk/python/feast/flags.py new file mode 100644 index 0000000000..38a12041af --- /dev/null +++ b/sdk/python/feast/flags.py @@ -0,0 +1,39 @@ +import os + +FLAG_ALPHA_FEATURES_NAME = "ENABLE_ALPHA_FEATURES" +FLAG_ON_DEMAND_TRANSFORM_NAME = "ENABLE_ON_DEMAND_TRANSFORMS" +FLAG_PYTHON_FEATURE_SERVER_NAME = "ENABLE_PYTHON_FEATURE_SERVER" +FLAG_IS_TEST = "IS_TEST" +FLAG_ENABLE_USAGE = "FEAST_USAGE" + + +def _flag_enabled(name: str) -> bool: + return os.getenv(name, default="False") == "True" + + +def _experimental_features_enabled() -> bool: + return _flag_enabled(FLAG_ALPHA_FEATURES_NAME) + + +def enable_usage() -> bool: + return _flag_enabled(FLAG_ENABLE_USAGE) + + +def is_test() -> bool: + return _flag_enabled(FLAG_IS_TEST) + + +def enable_on_demand_feature_views() -> bool: + if is_test(): + return True + return _experimental_features_enabled() and _flag_enabled( + FLAG_ON_DEMAND_TRANSFORM_NAME + ) + + +def enable_python_feature_server() -> bool: + if is_test(): + return True + return _experimental_features_enabled() and _flag_enabled( + FLAG_PYTHON_FEATURE_SERVER_NAME + ) diff --git a/sdk/python/feast/infra/offline_stores/bigquery.py b/sdk/python/feast/infra/offline_stores/bigquery.py index 0da4a329ec..ada25ac545 100644 --- a/sdk/python/feast/infra/offline_stores/bigquery.py +++ b/sdk/python/feast/infra/offline_stores/bigquery.py @@ -1,4 +1,3 @@ -import os import uuid from datetime import date, datetime, timedelta from typing import Dict, List, Optional, Union @@ -24,6 +23,7 @@ from feast.registry import Registry from feast.repo_config import FeastConfigBaseModel, RepoConfig +from ... import flags from .bigquery_source import BigQuerySource try: @@ -270,8 +270,7 @@ def block_until_done( """ # For test environments, retry more aggressively - is_test = os.getenv("IS_TEST", default="False") == "True" - if is_test: + if flags.is_test(): retry_cadence = 0.1 def _wait_until_done(bq_job): diff --git a/sdk/python/feast/usage.py b/sdk/python/feast/usage.py index ba6e95f32d..dc7bd84e26 100644 --- a/sdk/python/feast/usage.py +++ b/sdk/python/feast/usage.py @@ -23,6 +23,7 @@ import requests +from feast import flags from feast.version import get_version USAGE_ENDPOINT = "https://usage.feast.dev" @@ -35,9 +36,7 @@ def __init__(self): self.check_env_and_configure() def check_env_and_configure(self): - usage_enabled = ( - os.getenv("FEAST_USAGE", default="True") == "True" - ) # written this way to turn the env var string into a boolean + usage_enabled = flags.enable_usage() # Check if it changed if usage_enabled != self._usage_enabled: diff --git a/sdk/python/usage_tests/test_usage.py b/sdk/python/usage_tests/test_usage.py index e72e11b008..e291a8d430 100644 --- a/sdk/python/usage_tests/test_usage.py +++ b/sdk/python/usage_tests/test_usage.py @@ -22,8 +22,7 @@ import os from time import sleep -from feast import Entity, ValueType, FeatureStore, RepoConfig - +from feast import Entity, ValueType, FeatureStore, RepoConfig, flags USAGE_BIGQUERY_TABLE = ( "kf-feast.feast_telemetry.cloudfunctions_googleapis_com_cloud_functions" @@ -35,7 +34,7 @@ def test_usage_on(): test_usage_id = str(uuid.uuid4()) os.environ["FEAST_FORCE_USAGE_UUID"] = test_usage_id os.environ["FEAST_IS_USAGE_TEST"] = "True" - os.environ["FEAST_USAGE"] = "True" + os.environ[flags.FLAG_ENABLE_USAGE] = "True" with tempfile.TemporaryDirectory() as temp_dir: test_feature_store = FeatureStore( @@ -66,7 +65,7 @@ def test_usage_off(): old_environ = dict(os.environ) test_usage_id = str(uuid.uuid4()) os.environ["FEAST_IS_USAGE_TEST"] = "True" - os.environ["FEAST_USAGE"] = "False" + os.environ[flags.FLAG_ENABLE_USAGE] = "False" os.environ["FEAST_FORCE_USAGE_UUID"] = test_usage_id with tempfile.TemporaryDirectory() as temp_dir: @@ -100,7 +99,7 @@ def test_exception_usage_on(): test_usage_id = str(uuid.uuid4()) os.environ["FEAST_FORCE_USAGE_UUID"] = test_usage_id os.environ["FEAST_IS_USAGE_TEST"] = "True" - os.environ["FEAST_USAGE"] = "True" + os.environ[flags.FLAG_ENABLE_USAGE] = "True" try: test_feature_store = FeatureStore("/tmp/non_existent_directory") @@ -116,7 +115,7 @@ def test_exception_usage_off(): old_environ = dict(os.environ) test_usage_id = str(uuid.uuid4()) os.environ["FEAST_IS_USAGE_TEST"] = "True" - os.environ["FEAST_USAGE"] = "False" + os.environ[flags.FLAG_ENABLE_USAGE] = "False" os.environ["FEAST_FORCE_USAGE_UUID"] = test_usage_id try: From 841980aa392636f2d315f3610dc5957a34b83994 Mon Sep 17 00:00:00 2001 From: Danny Chiao Date: Wed, 15 Sep 2021 15:03:34 -0400 Subject: [PATCH 02/10] Address comments and enable subfeature flags by default Signed-off-by: Danny Chiao --- sdk/python/feast/errors.py | 2 +- sdk/python/feast/flags.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/sdk/python/feast/errors.py b/sdk/python/feast/errors.py index 6ab6d82edd..05539695b9 100644 --- a/sdk/python/feast/errors.py +++ b/sdk/python/feast/errors.py @@ -265,5 +265,5 @@ def __init__(self, feature_view_name: str): class ExperimentalFeatureNotEnabled(Exception): def __init__(self, feature_flag_name: str): super().__init__( - f"Trying to use experimental feature. Please ensure to set environment variables {FLAG_ALPHA_FEATURES_NAME} and {feature_flag_name} to true" + f"You are attempting to use an experimental feature. Please set the environment variables {FLAG_ALPHA_FEATURES_NAME} and {feature_flag_name} to true" ) diff --git a/sdk/python/feast/flags.py b/sdk/python/feast/flags.py index 38a12041af..e2b8081d5e 100644 --- a/sdk/python/feast/flags.py +++ b/sdk/python/feast/flags.py @@ -7,8 +7,8 @@ FLAG_ENABLE_USAGE = "FEAST_USAGE" -def _flag_enabled(name: str) -> bool: - return os.getenv(name, default="False") == "True" +def _flag_enabled(name: str, enable_by_default: bool = False) -> bool: + return os.getenv(name, default=str(enable_by_default)) == "True" def _experimental_features_enabled() -> bool: @@ -27,7 +27,7 @@ def enable_on_demand_feature_views() -> bool: if is_test(): return True return _experimental_features_enabled() and _flag_enabled( - FLAG_ON_DEMAND_TRANSFORM_NAME + FLAG_ON_DEMAND_TRANSFORM_NAME, enable_by_default=True ) @@ -35,5 +35,5 @@ def enable_python_feature_server() -> bool: if is_test(): return True return _experimental_features_enabled() and _flag_enabled( - FLAG_PYTHON_FEATURE_SERVER_NAME + FLAG_PYTHON_FEATURE_SERVER_NAME, enable_by_default=True ) From 4249ed4f56db28626f18026c158e41c1c725d12f Mon Sep 17 00:00:00 2001 From: Danny Chiao Date: Wed, 15 Sep 2021 16:30:28 -0400 Subject: [PATCH 03/10] Rewriting to use yaml as source of truth for feature flags Signed-off-by: Danny Chiao --- sdk/python/feast/cli.py | 63 ++++++++++++++++--- sdk/python/feast/errors.py | 2 +- sdk/python/feast/feature_store.py | 9 ++- sdk/python/feast/flags.py | 50 ++++----------- sdk/python/feast/flags_helper.py | 34 ++++++++++ .../feast/infra/offline_stores/bigquery.py | 4 +- sdk/python/feast/repo_config.py | 23 +++++++ sdk/python/feast/usage.py | 3 +- 8 files changed, 133 insertions(+), 55 deletions(-) create mode 100644 sdk/python/feast/flags_helper.py diff --git a/sdk/python/feast/cli.py b/sdk/python/feast/cli.py index 14d60864bd..ad1bf66235 100644 --- a/sdk/python/feast/cli.py +++ b/sdk/python/feast/cli.py @@ -22,11 +22,7 @@ import yaml from feast import flags, utils -from feast.errors import ( - ExperimentalFeatureNotEnabled, - FeastObjectNotFoundException, - FeastProviderLoginError, -) +from feast.errors import FeastObjectNotFoundException, FeastProviderLoginError from feast.feature_store import FeatureStore from feast.repo_config import load_repo_config from feast.repo_operations import ( @@ -420,9 +416,6 @@ def init_command(project_directory, minimal: bool, template: str): @click.pass_context def serve_command(ctx: click.Context, port: int): """[Experimental] Start a the feature consumption server locally on a given port.""" - if not flags.enable_python_feature_server(): - raise ExperimentalFeatureNotEnabled(flags.FLAG_PYTHON_FEATURE_SERVER_NAME) - repo = ctx.obj["CHDIR"] cli_check_repo(repo) store = FeatureStore(repo_path=str(repo)) @@ -430,5 +423,59 @@ def serve_command(ctx: click.Context, port: int): store.serve(port) +@cli.command("enable-alpha-feature") +@click.argument("name", type=click.STRING) +@click.pass_context +def enable_alpha_feature(ctx: click.Context, name: str): + """ + Enables an alpha feature + """ + if name not in flags.FLAG_NAMES: + raise ValueError(f"Flag name, {name}, not valid.") + + repo = ctx.obj["CHDIR"] + cli_check_repo(repo) + repo_path = str(repo) + store = FeatureStore(repo_path=repo_path) + + store.config.flags[flags.FLAG_ALPHA_FEATURES_NAME] = True + store.config.flags[name] = True + store.config.write_to_path(Path(repo_path)) + + +@cli.command("disable-alpha-feature") +@click.argument("name", type=click.STRING) +@click.pass_context +def disable_alpha_feature(ctx: click.Context, name: str): + """ + Disables an alpha feature + """ + if name not in flags.FLAG_NAMES: + raise ValueError(f"Flag name, {name}, not valid.") + + repo = ctx.obj["CHDIR"] + cli_check_repo(repo) + repo_path = str(repo) + store = FeatureStore(repo_path=repo_path) + + store.config.flags[name] = False + store.config.write_to_path(Path(repo_path)) + + +@cli.command("disable-alpha-features") +@click.pass_context +def disable_alpha_features(ctx: click.Context): + """ + Disables all alpha features + """ + repo = ctx.obj["CHDIR"] + cli_check_repo(repo) + repo_path = str(repo) + store = FeatureStore(repo_path=repo_path) + + store.config.flags = None + store.config.write_to_path(Path(repo_path)) + + if __name__ == "__main__": cli() diff --git a/sdk/python/feast/errors.py b/sdk/python/feast/errors.py index 05539695b9..73f10c5bf3 100644 --- a/sdk/python/feast/errors.py +++ b/sdk/python/feast/errors.py @@ -265,5 +265,5 @@ def __init__(self, feature_view_name: str): class ExperimentalFeatureNotEnabled(Exception): def __init__(self, feature_flag_name: str): super().__init__( - f"You are attempting to use an experimental feature. Please set the environment variables {FLAG_ALPHA_FEATURES_NAME} and {feature_flag_name} to true" + f"You are attempting to use an experimental feature. Please enable the environment variables {FLAG_ALPHA_FEATURES_NAME} and {feature_flag_name} to true" ) diff --git a/sdk/python/feast/feature_store.py b/sdk/python/feast/feature_store.py index 471fb85b97..dc4d4262b6 100644 --- a/sdk/python/feast/feature_store.py +++ b/sdk/python/feast/feature_store.py @@ -22,7 +22,7 @@ from colorama import Fore, Style from tqdm import tqdm -from feast import feature_server, flags, utils +from feast import feature_server, flags, flags_helper, utils from feast.data_source import RequestDataSource from feast.entity import Entity from feast.errors import ( @@ -381,7 +381,10 @@ def apply( views_to_update = [ob for ob in objects if isinstance(ob, FeatureView)] odfvs_to_update = [ob for ob in objects if isinstance(ob, OnDemandFeatureView)] - if not flags.enable_on_demand_feature_views() and len(odfvs_to_update) > 0: + if ( + not flags_helper.enable_on_demand_feature_views(self.config) + and len(odfvs_to_update) > 0 + ): raise ExperimentalFeatureNotEnabled(flags.FLAG_ON_DEMAND_TRANSFORM_NAME) _validate_feature_views(views_to_update) @@ -990,7 +993,7 @@ def _augment_response_with_on_demand_transforms( @log_exceptions_and_usage def serve(self, port: int) -> None: """Start the feature consumption server locally on a given port.""" - if not flags.enable_python_feature_server(): + if not flags_helper.enable_python_feature_server(self.config): raise ExperimentalFeatureNotEnabled(flags.FLAG_PYTHON_FEATURE_SERVER_NAME) feature_server.start_server(self, port) diff --git a/sdk/python/feast/flags.py b/sdk/python/feast/flags.py index e2b8081d5e..c2426f7f21 100644 --- a/sdk/python/feast/flags.py +++ b/sdk/python/feast/flags.py @@ -1,39 +1,11 @@ -import os - -FLAG_ALPHA_FEATURES_NAME = "ENABLE_ALPHA_FEATURES" -FLAG_ON_DEMAND_TRANSFORM_NAME = "ENABLE_ON_DEMAND_TRANSFORMS" -FLAG_PYTHON_FEATURE_SERVER_NAME = "ENABLE_PYTHON_FEATURE_SERVER" -FLAG_IS_TEST = "IS_TEST" -FLAG_ENABLE_USAGE = "FEAST_USAGE" - - -def _flag_enabled(name: str, enable_by_default: bool = False) -> bool: - return os.getenv(name, default=str(enable_by_default)) == "True" - - -def _experimental_features_enabled() -> bool: - return _flag_enabled(FLAG_ALPHA_FEATURES_NAME) - - -def enable_usage() -> bool: - return _flag_enabled(FLAG_ENABLE_USAGE) - - -def is_test() -> bool: - return _flag_enabled(FLAG_IS_TEST) - - -def enable_on_demand_feature_views() -> bool: - if is_test(): - return True - return _experimental_features_enabled() and _flag_enabled( - FLAG_ON_DEMAND_TRANSFORM_NAME, enable_by_default=True - ) - - -def enable_python_feature_server() -> bool: - if is_test(): - return True - return _experimental_features_enabled() and _flag_enabled( - FLAG_PYTHON_FEATURE_SERVER_NAME, enable_by_default=True - ) +FLAG_ALPHA_FEATURES_NAME = "enable_alpha_features" +FLAG_ON_DEMAND_TRANSFORM_NAME = "enable_on_demand_transforms" +FLAG_PYTHON_FEATURE_SERVER_NAME = "enable_python_feature_server" +ENV_FLAG_IS_TEST = "IS_TEST" +ENV_FLAG_ENABLE_USAGE = "FEAST_USAGE" + +FLAG_NAMES = { + FLAG_ALPHA_FEATURES_NAME, + FLAG_ON_DEMAND_TRANSFORM_NAME, + FLAG_PYTHON_FEATURE_SERVER_NAME, +} diff --git a/sdk/python/feast/flags_helper.py b/sdk/python/feast/flags_helper.py new file mode 100644 index 0000000000..91eafdebf6 --- /dev/null +++ b/sdk/python/feast/flags_helper.py @@ -0,0 +1,34 @@ +import os + +from feast import flags +from feast.repo_config import RepoConfig + + +def _env_flag_enabled(name: str) -> bool: + return os.getenv(name, default="False") == "True" + + +def _experimental_features_enabled(repo_config: RepoConfig) -> bool: + return repo_config.flags[flags.FLAG_ALPHA_FEATURES_NAME] + + +def is_test() -> bool: + return _env_flag_enabled(flags.ENV_FLAG_IS_TEST) + + +def enable_on_demand_feature_views(repo_config: RepoConfig) -> bool: + if is_test(): + return True + return ( + _experimental_features_enabled(repo_config) + and repo_config.flags[flags.FLAG_ON_DEMAND_TRANSFORM_NAME] + ) + + +def enable_python_feature_server(repo_config: RepoConfig) -> bool: + if is_test(): + return True + return ( + _experimental_features_enabled(repo_config) + and repo_config.flags[flags.FLAG_PYTHON_FEATURE_SERVER_NAME] + ) diff --git a/sdk/python/feast/infra/offline_stores/bigquery.py b/sdk/python/feast/infra/offline_stores/bigquery.py index ada25ac545..c3c5096d96 100644 --- a/sdk/python/feast/infra/offline_stores/bigquery.py +++ b/sdk/python/feast/infra/offline_stores/bigquery.py @@ -9,6 +9,7 @@ from pydantic.typing import Literal from tenacity import Retrying, retry_if_exception_type, stop_after_delay, wait_fixed +from feast import flags_helper from feast.data_source import DataSource from feast.errors import ( BigQueryJobCancelled, @@ -23,7 +24,6 @@ from feast.registry import Registry from feast.repo_config import FeastConfigBaseModel, RepoConfig -from ... import flags from .bigquery_source import BigQuerySource try: @@ -270,7 +270,7 @@ def block_until_done( """ # For test environments, retry more aggressively - if flags.is_test(): + if flags_helper.is_test(): retry_cadence = 0.1 def _wait_until_done(bq_job): diff --git a/sdk/python/feast/repo_config.py b/sdk/python/feast/repo_config.py index b2a2d913ea..34e107e142 100644 --- a/sdk/python/feast/repo_config.py +++ b/sdk/python/feast/repo_config.py @@ -13,6 +13,7 @@ from pydantic.error_wrappers import ErrorWrapper from pydantic.typing import Dict, Optional, Union +from feast import flags from feast.errors import ( FeastFeatureServerTypeInvalidError, FeastFeatureServerTypeSetError, @@ -98,6 +99,10 @@ class RepoConfig(FeastBaseModel): feature_server: Optional[Any] """ FeatureServerConfig: Feature server configuration (optional depending on provider) """ + flags: Any + """ Flags: feature flags for experimental features (optional depending on provider) """ + + repo_path: Optional[Path] = None def __init__(self, **data: Any): @@ -255,6 +260,24 @@ def _validate_project_name(cls, v): ) return v + @validator("flags") + def _validate_flags(cls, v): + if not isinstance(v, Dict): + return + + for flag_name, val in v.items(): + if flag_name not in flags.FLAG_NAMES: + raise ValueError(f"Flag name, {flag_name}, not valid.") + if type(val) is not bool: + raise ValueError(f"Flag value, {val}, not valid.") + + return v + + def write_to_path(self, repo_path: Path): + config_path = repo_path / "feature_store.yaml" + with open(config_path, mode="w") as f: + yaml.dump(yaml.safe_load(self.json()), f) + class FeastConfigError(Exception): def __init__(self, error_message, config_path): diff --git a/sdk/python/feast/usage.py b/sdk/python/feast/usage.py index dc7bd84e26..efe241e699 100644 --- a/sdk/python/feast/usage.py +++ b/sdk/python/feast/usage.py @@ -23,7 +23,6 @@ import requests -from feast import flags from feast.version import get_version USAGE_ENDPOINT = "https://usage.feast.dev" @@ -36,7 +35,7 @@ def __init__(self): self.check_env_and_configure() def check_env_and_configure(self): - usage_enabled = flags.enable_usage() + usage_enabled = os.getenv("FEAST_USAGE", default="True") == "True" # Check if it changed if usage_enabled != self._usage_enabled: From 113138974887018e4a6b09901bb599eba04fa2cb Mon Sep 17 00:00:00 2001 From: Danny Chiao Date: Wed, 15 Sep 2021 17:15:36 -0400 Subject: [PATCH 04/10] Fixing defaults Signed-off-by: Danny Chiao --- sdk/python/feast/cli.py | 47 ++++++++++++++++++++++++++++++-- sdk/python/feast/errors.py | 4 +-- sdk/python/feast/flags_helper.py | 26 ++++++++++++------ sdk/python/feast/repo_config.py | 12 +++++++- 4 files changed, 74 insertions(+), 15 deletions(-) diff --git a/sdk/python/feast/cli.py b/sdk/python/feast/cli.py index ad1bf66235..609720e0f0 100644 --- a/sdk/python/feast/cli.py +++ b/sdk/python/feast/cli.py @@ -423,7 +423,44 @@ def serve_command(ctx: click.Context, port: int): store.serve(port) -@cli.command("enable-alpha-feature") +@cli.group(name="alpha") +def alpha_cmd(): + """ + Access alpha features + """ + pass + + +@alpha_cmd.command("list") +def list_alpha_features(): + """ + Lists all alpha features + """ + flags_to_show = flags.FLAG_NAMES.copy() + flags_to_show.remove(flags.FLAG_ALPHA_FEATURES_NAME) + print("Alpha features:") + print(*flags_to_show, sep="\n") + + +@alpha_cmd.command("enable-all") +@click.pass_context +def enable_alpha_features(ctx: click.Context): + """ + Enables all alpha features + """ + repo = ctx.obj["CHDIR"] + cli_check_repo(repo) + repo_path = str(repo) + store = FeatureStore(repo_path=repo_path) + + if store.config.flags is None: + store.config.flags = {} + for flag_name in flags.FLAG_NAMES: + store.config.flags[flag_name] = True + store.config.write_to_path(Path(repo_path)) + + +@alpha_cmd.command("enable") @click.argument("name", type=click.STRING) @click.pass_context def enable_alpha_feature(ctx: click.Context, name: str): @@ -438,12 +475,14 @@ def enable_alpha_feature(ctx: click.Context, name: str): repo_path = str(repo) store = FeatureStore(repo_path=repo_path) + if store.config.flags is None: + store.config.flags = {} store.config.flags[flags.FLAG_ALPHA_FEATURES_NAME] = True store.config.flags[name] = True store.config.write_to_path(Path(repo_path)) -@cli.command("disable-alpha-feature") +@alpha_cmd.command("disable") @click.argument("name", type=click.STRING) @click.pass_context def disable_alpha_feature(ctx: click.Context, name: str): @@ -458,11 +497,13 @@ def disable_alpha_feature(ctx: click.Context, name: str): repo_path = str(repo) store = FeatureStore(repo_path=repo_path) + if store.config.flags is None or name not in store.config.flags: + return store.config.flags[name] = False store.config.write_to_path(Path(repo_path)) -@cli.command("disable-alpha-features") +@alpha_cmd.command("disable-all") @click.pass_context def disable_alpha_features(ctx: click.Context): """ diff --git a/sdk/python/feast/errors.py b/sdk/python/feast/errors.py index 73f10c5bf3..9797ddf808 100644 --- a/sdk/python/feast/errors.py +++ b/sdk/python/feast/errors.py @@ -2,8 +2,6 @@ from colorama import Fore, Style -from feast.flags import FLAG_ALPHA_FEATURES_NAME - class DataSourceNotFoundException(Exception): def __init__(self, path): @@ -265,5 +263,5 @@ def __init__(self, feature_view_name: str): class ExperimentalFeatureNotEnabled(Exception): def __init__(self, feature_flag_name: str): super().__init__( - f"You are attempting to use an experimental feature. Please enable the environment variables {FLAG_ALPHA_FEATURES_NAME} and {feature_flag_name} to true" + f"You are attempting to use an experimental feature. Please run `feast alpha enable {feature_flag_name}`" ) diff --git a/sdk/python/feast/flags_helper.py b/sdk/python/feast/flags_helper.py index 91eafdebf6..8d68cfca0f 100644 --- a/sdk/python/feast/flags_helper.py +++ b/sdk/python/feast/flags_helper.py @@ -8,8 +8,20 @@ def _env_flag_enabled(name: str) -> bool: return os.getenv(name, default="False") == "True" -def _experimental_features_enabled(repo_config: RepoConfig) -> bool: - return repo_config.flags[flags.FLAG_ALPHA_FEATURES_NAME] +def _feature_flag_enabled(repo_config: RepoConfig, flag_name: str) -> bool: + return ( + repo_config.flags is not None + and flag_name in repo_config.flags + and repo_config.flags[flag_name] + ) + + +def _alpha_feature_flag_enabled(repo_config: RepoConfig) -> bool: + return ( + repo_config.flags is not None + and flags.FLAG_ALPHA_FEATURES_NAME in repo_config.flags + and repo_config.flags[flags.FLAG_ALPHA_FEATURES_NAME] + ) def is_test() -> bool: @@ -19,16 +31,14 @@ def is_test() -> bool: def enable_on_demand_feature_views(repo_config: RepoConfig) -> bool: if is_test(): return True - return ( - _experimental_features_enabled(repo_config) - and repo_config.flags[flags.FLAG_ON_DEMAND_TRANSFORM_NAME] + return _alpha_feature_flag_enabled(repo_config) and _feature_flag_enabled( + repo_config, flags.FLAG_ON_DEMAND_TRANSFORM_NAME ) def enable_python_feature_server(repo_config: RepoConfig) -> bool: if is_test(): return True - return ( - _experimental_features_enabled(repo_config) - and repo_config.flags[flags.FLAG_PYTHON_FEATURE_SERVER_NAME] + return _alpha_feature_flag_enabled(repo_config) and _feature_flag_enabled( + repo_config, flags.FLAG_PYTHON_FEATURE_SERVER_NAME ) diff --git a/sdk/python/feast/repo_config.py b/sdk/python/feast/repo_config.py index 34e107e142..5c259b3eea 100644 --- a/sdk/python/feast/repo_config.py +++ b/sdk/python/feast/repo_config.py @@ -276,7 +276,17 @@ def _validate_flags(cls, v): def write_to_path(self, repo_path: Path): config_path = repo_path / "feature_store.yaml" with open(config_path, mode="w") as f: - yaml.dump(yaml.safe_load(self.json()), f) + yaml.dump( + yaml.safe_load( + self.json( + exclude={"repo_path"}, + exclude_none=True, + exclude_unset=True, + exclude_defaults=True, + ) + ), + f, + ) class FeastConfigError(Exception): From 459af6f3935eac7bf5ef551ed4685dc71f50f218 Mon Sep 17 00:00:00 2001 From: Danny Chiao Date: Wed, 15 Sep 2021 17:19:11 -0400 Subject: [PATCH 05/10] Revert usage change Signed-off-by: Danny Chiao --- sdk/python/feast/usage.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sdk/python/feast/usage.py b/sdk/python/feast/usage.py index efe241e699..ba6e95f32d 100644 --- a/sdk/python/feast/usage.py +++ b/sdk/python/feast/usage.py @@ -35,7 +35,9 @@ def __init__(self): self.check_env_and_configure() def check_env_and_configure(self): - usage_enabled = os.getenv("FEAST_USAGE", default="True") == "True" + usage_enabled = ( + os.getenv("FEAST_USAGE", default="True") == "True" + ) # written this way to turn the env var string into a boolean # Check if it changed if usage_enabled != self._usage_enabled: From dafb64eb5d72f3902f131e24242a6078eb4a3b80 Mon Sep 17 00:00:00 2001 From: Danny Chiao Date: Wed, 15 Sep 2021 17:21:00 -0400 Subject: [PATCH 06/10] Revert usage change 2 Signed-off-by: Danny Chiao --- sdk/python/feast/flags.py | 1 - sdk/python/usage_tests/test_usage.py | 10 +++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/sdk/python/feast/flags.py b/sdk/python/feast/flags.py index c2426f7f21..bda615ca0e 100644 --- a/sdk/python/feast/flags.py +++ b/sdk/python/feast/flags.py @@ -2,7 +2,6 @@ FLAG_ON_DEMAND_TRANSFORM_NAME = "enable_on_demand_transforms" FLAG_PYTHON_FEATURE_SERVER_NAME = "enable_python_feature_server" ENV_FLAG_IS_TEST = "IS_TEST" -ENV_FLAG_ENABLE_USAGE = "FEAST_USAGE" FLAG_NAMES = { FLAG_ALPHA_FEATURES_NAME, diff --git a/sdk/python/usage_tests/test_usage.py b/sdk/python/usage_tests/test_usage.py index e291a8d430..0f90fd5c99 100644 --- a/sdk/python/usage_tests/test_usage.py +++ b/sdk/python/usage_tests/test_usage.py @@ -22,7 +22,7 @@ import os from time import sleep -from feast import Entity, ValueType, FeatureStore, RepoConfig, flags +from feast import Entity, ValueType, FeatureStore, RepoConfig USAGE_BIGQUERY_TABLE = ( "kf-feast.feast_telemetry.cloudfunctions_googleapis_com_cloud_functions" @@ -34,7 +34,7 @@ def test_usage_on(): test_usage_id = str(uuid.uuid4()) os.environ["FEAST_FORCE_USAGE_UUID"] = test_usage_id os.environ["FEAST_IS_USAGE_TEST"] = "True" - os.environ[flags.FLAG_ENABLE_USAGE] = "True" + os.environ["FEAST_USAGE"] = "True" with tempfile.TemporaryDirectory() as temp_dir: test_feature_store = FeatureStore( @@ -65,7 +65,7 @@ def test_usage_off(): old_environ = dict(os.environ) test_usage_id = str(uuid.uuid4()) os.environ["FEAST_IS_USAGE_TEST"] = "True" - os.environ[flags.FLAG_ENABLE_USAGE] = "False" + os.environ["FEAST_USAGE"] = "False" os.environ["FEAST_FORCE_USAGE_UUID"] = test_usage_id with tempfile.TemporaryDirectory() as temp_dir: @@ -99,7 +99,7 @@ def test_exception_usage_on(): test_usage_id = str(uuid.uuid4()) os.environ["FEAST_FORCE_USAGE_UUID"] = test_usage_id os.environ["FEAST_IS_USAGE_TEST"] = "True" - os.environ[flags.FLAG_ENABLE_USAGE] = "True" + os.environ["FEAST_USAGE"] = "True" try: test_feature_store = FeatureStore("/tmp/non_existent_directory") @@ -115,7 +115,7 @@ def test_exception_usage_off(): old_environ = dict(os.environ) test_usage_id = str(uuid.uuid4()) os.environ["FEAST_IS_USAGE_TEST"] = "True" - os.environ[flags.FLAG_ENABLE_USAGE] = "False" + os.environ["FEAST_USAGE"] = "False" os.environ["FEAST_FORCE_USAGE_UUID"] = test_usage_id try: From 7de3c7f3b784710999d514f736e160e7c5b669fb Mon Sep 17 00:00:00 2001 From: Danny Chiao Date: Wed, 15 Sep 2021 17:21:26 -0400 Subject: [PATCH 07/10] Revert usage change 3 Signed-off-by: Danny Chiao --- sdk/python/usage_tests/test_usage.py | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/python/usage_tests/test_usage.py b/sdk/python/usage_tests/test_usage.py index 0f90fd5c99..e72e11b008 100644 --- a/sdk/python/usage_tests/test_usage.py +++ b/sdk/python/usage_tests/test_usage.py @@ -24,6 +24,7 @@ from feast import Entity, ValueType, FeatureStore, RepoConfig + USAGE_BIGQUERY_TABLE = ( "kf-feast.feast_telemetry.cloudfunctions_googleapis_com_cloud_functions" ) From 282199f63b33f0cc9ae4f0cef03c4cdc6d989ff0 Mon Sep 17 00:00:00 2001 From: Danny Chiao Date: Wed, 15 Sep 2021 17:57:11 -0400 Subject: [PATCH 08/10] Fix description of flags member Signed-off-by: Danny Chiao --- sdk/python/feast/repo_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/python/feast/repo_config.py b/sdk/python/feast/repo_config.py index 5c259b3eea..66c14dd737 100644 --- a/sdk/python/feast/repo_config.py +++ b/sdk/python/feast/repo_config.py @@ -100,7 +100,7 @@ class RepoConfig(FeastBaseModel): """ FeatureServerConfig: Feature server configuration (optional depending on provider) """ flags: Any - """ Flags: feature flags for experimental features (optional depending on provider) """ + """ Flags: Feature flags for experimental features (optional) """ repo_path: Optional[Path] = None From 58303b7b5d00f5a716528f37ba86a708f3859fad Mon Sep 17 00:00:00 2001 From: Danny Chiao Date: Wed, 15 Sep 2021 18:03:13 -0400 Subject: [PATCH 09/10] Not sorting yaml order when serializing Signed-off-by: Danny Chiao --- sdk/python/feast/repo_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/python/feast/repo_config.py b/sdk/python/feast/repo_config.py index 66c14dd737..35becb5641 100644 --- a/sdk/python/feast/repo_config.py +++ b/sdk/python/feast/repo_config.py @@ -102,7 +102,6 @@ class RepoConfig(FeastBaseModel): flags: Any """ Flags: Feature flags for experimental features (optional) """ - repo_path: Optional[Path] = None def __init__(self, **data: Any): @@ -286,6 +285,7 @@ def write_to_path(self, repo_path: Path): ) ), f, + sort_keys=False, ) From ca8707d8f0a21fa286bc2683752c7f28823f65d2 Mon Sep 17 00:00:00 2001 From: Danny Chiao Date: Thu, 16 Sep 2021 15:17:54 -0400 Subject: [PATCH 10/10] Comments Signed-off-by: Danny Chiao --- sdk/python/feast/cli.py | 18 +++++++++++++++--- sdk/python/feast/errors.py | 3 ++- sdk/python/feast/flags.py | 6 +++--- sdk/python/feast/flags_helper.py | 19 +++++++------------ 4 files changed, 27 insertions(+), 19 deletions(-) diff --git a/sdk/python/feast/cli.py b/sdk/python/feast/cli.py index 609720e0f0..960afc76f7 100644 --- a/sdk/python/feast/cli.py +++ b/sdk/python/feast/cli.py @@ -21,7 +21,7 @@ import pkg_resources import yaml -from feast import flags, utils +from feast import flags, flags_helper, utils from feast.errors import FeastObjectNotFoundException, FeastProviderLoginError from feast.feature_store import FeatureStore from feast.repo_config import load_repo_config @@ -432,14 +432,26 @@ def alpha_cmd(): @alpha_cmd.command("list") -def list_alpha_features(): +@click.pass_context +def list_alpha_features(ctx: click.Context): """ Lists all alpha features """ + repo = ctx.obj["CHDIR"] + cli_check_repo(repo) + repo_path = str(repo) + store = FeatureStore(repo_path=repo_path) + flags_to_show = flags.FLAG_NAMES.copy() flags_to_show.remove(flags.FLAG_ALPHA_FEATURES_NAME) print("Alpha features:") - print(*flags_to_show, sep="\n") + for flag in flags_to_show: + enabled_string = ( + "enabled" + if flags_helper.feature_flag_enabled(store.config, flag) + else "disabled" + ) + print(f"{flag}: {enabled_string}") @alpha_cmd.command("enable-all") diff --git a/sdk/python/feast/errors.py b/sdk/python/feast/errors.py index 9797ddf808..0d4fb929d9 100644 --- a/sdk/python/feast/errors.py +++ b/sdk/python/feast/errors.py @@ -263,5 +263,6 @@ def __init__(self, feature_view_name: str): class ExperimentalFeatureNotEnabled(Exception): def __init__(self, feature_flag_name: str): super().__init__( - f"You are attempting to use an experimental feature. Please run `feast alpha enable {feature_flag_name}`" + f"You are attempting to use an experimental feature that is not enabled. Please run " + f"`feast alpha enable {feature_flag_name}` " ) diff --git a/sdk/python/feast/flags.py b/sdk/python/feast/flags.py index bda615ca0e..67c50057cb 100644 --- a/sdk/python/feast/flags.py +++ b/sdk/python/feast/flags.py @@ -1,6 +1,6 @@ -FLAG_ALPHA_FEATURES_NAME = "enable_alpha_features" -FLAG_ON_DEMAND_TRANSFORM_NAME = "enable_on_demand_transforms" -FLAG_PYTHON_FEATURE_SERVER_NAME = "enable_python_feature_server" +FLAG_ALPHA_FEATURES_NAME = "alpha_features" +FLAG_ON_DEMAND_TRANSFORM_NAME = "on_demand_transforms" +FLAG_PYTHON_FEATURE_SERVER_NAME = "python_feature_server" ENV_FLAG_IS_TEST = "IS_TEST" FLAG_NAMES = { diff --git a/sdk/python/feast/flags_helper.py b/sdk/python/feast/flags_helper.py index 8d68cfca0f..a8f3373e1e 100644 --- a/sdk/python/feast/flags_helper.py +++ b/sdk/python/feast/flags_helper.py @@ -8,9 +8,12 @@ def _env_flag_enabled(name: str) -> bool: return os.getenv(name, default="False") == "True" -def _feature_flag_enabled(repo_config: RepoConfig, flag_name: str) -> bool: +def feature_flag_enabled(repo_config: RepoConfig, flag_name: str) -> bool: + if is_test(): + return True return ( - repo_config.flags is not None + _alpha_feature_flag_enabled(repo_config) + and repo_config.flags is not None and flag_name in repo_config.flags and repo_config.flags[flag_name] ) @@ -29,16 +32,8 @@ def is_test() -> bool: def enable_on_demand_feature_views(repo_config: RepoConfig) -> bool: - if is_test(): - return True - return _alpha_feature_flag_enabled(repo_config) and _feature_flag_enabled( - repo_config, flags.FLAG_ON_DEMAND_TRANSFORM_NAME - ) + return feature_flag_enabled(repo_config, flags.FLAG_ON_DEMAND_TRANSFORM_NAME) def enable_python_feature_server(repo_config: RepoConfig) -> bool: - if is_test(): - return True - return _alpha_feature_flag_enabled(repo_config) and _feature_flag_enabled( - repo_config, flags.FLAG_PYTHON_FEATURE_SERVER_NAME - ) + return feature_flag_enabled(repo_config, flags.FLAG_PYTHON_FEATURE_SERVER_NAME)