Skip to content

Commit

Permalink
chore: Remove go server references (feast-dev#3554)
Browse files Browse the repository at this point in the history
* chore: Remove go server callsites

Signed-off-by: Achal Shah <achals@gmail.com>

* remove more go traces

Signed-off-by: Achal Shah <achals@gmail.com>

* remove more go traces

Signed-off-by: Achal Shah <achals@gmail.com>

---------

Signed-off-by: Achal Shah <achals@gmail.com>
  • Loading branch information
achals authored Mar 24, 2023
1 parent a76c6d0 commit 5e62844
Show file tree
Hide file tree
Showing 8 changed files with 9 additions and 403 deletions.
11 changes: 0 additions & 11 deletions sdk/python/feast/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -684,12 +684,6 @@ def init_command(project_directory, minimal: bool, template: str):
show_default=True,
help="Specify a server type: 'http' or 'grpc'",
)
@click.option(
"--go",
is_flag=True,
show_default=True,
help="Use Go to serve",
)
@click.option(
"--no-access-log",
is_flag=True,
Expand All @@ -708,7 +702,6 @@ def serve_command(
host: str,
port: int,
type_: str,
go: bool,
no_access_log: bool,
no_feature_log: bool,
):
Expand All @@ -730,10 +723,6 @@ def serve_command(
cli_check_repo(repo, fs_yaml_file)
store = FeatureStore(repo_path=str(repo), fs_yaml_file=fs_yaml_file)

if go:
# Turn on Go feature retrieval.
store.config.go_feature_serving = True

store.serve(host, port, type_, no_access_log, no_feature_log)


Expand Down
105 changes: 6 additions & 99 deletions sdk/python/feast/feature_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
from datetime import datetime, timedelta
from pathlib import Path
from typing import (
TYPE_CHECKING,
Any,
Callable,
Dict,
Expand Down Expand Up @@ -94,19 +93,13 @@
from feast.request_feature_view import RequestFeatureView
from feast.saved_dataset import SavedDataset, SavedDatasetStorage, ValidationReference
from feast.stream_feature_view import StreamFeatureView
from feast.type_map import (
feast_value_type_to_python_type,
python_values_to_proto_values,
)
from feast.type_map import python_values_to_proto_values
from feast.usage import log_exceptions, log_exceptions_and_usage, set_usage_attribute
from feast.value_type import ValueType
from feast.version import get_version

warnings.simplefilter("once", DeprecationWarning)

if TYPE_CHECKING:
from feast.embedded_go.online_features_service import EmbeddedOnlineFeatureServer


class FeatureStore:
"""
Expand All @@ -117,14 +110,12 @@ class FeatureStore:
repo_path: The path to the feature repo.
_registry: The registry for the feature store.
_provider: The provider for the feature store.
_go_server: The (optional) Go feature server for the feature store.
"""

config: RepoConfig
repo_path: Path
_registry: BaseRegistry
_provider: Provider
_go_server: Optional["EmbeddedOnlineFeatureServer"]

@log_exceptions
def __init__(
Expand Down Expand Up @@ -179,7 +170,6 @@ def __init__(
self._registry = r

self._provider = get_provider(self.config)
self._go_server = None

@log_exceptions
def version(self) -> str:
Expand Down Expand Up @@ -1009,11 +999,6 @@ def apply(

self._registry.commit()

# go server needs to be reloaded to apply new configuration.
# we're stopping it here
# new server will be instantiated on the next online request
self._teardown_go_server()

@log_exceptions_and_usage
def teardown(self):
"""Tears down all local and cloud resources for the feature store."""
Expand All @@ -1026,7 +1011,6 @@ def teardown(self):

self._get_provider().teardown_infra(self.project, tables, entities)
self._registry.teardown()
self._teardown_go_server()

@log_exceptions_and_usage
def get_historical_features(
Expand Down Expand Up @@ -1603,18 +1587,6 @@ def get_online_features(
native_entity_values=True,
)

def _lazy_init_go_server(self):
"""Lazily initialize self._go_server if it hasn't been initialized before."""
from feast.embedded_go.online_features_service import (
EmbeddedOnlineFeatureServer,
)

# Lazily start the go server on the first request
if self._go_server is None:
self._go_server = EmbeddedOnlineFeatureServer(
str(self.repo_path.absolute()), self.config, self
)

def _get_online_features(
self,
features: Union[List[str], FeatureService],
Expand All @@ -1630,35 +1602,6 @@ def _get_online_features(
for k, v in entity_values.items()
}

# If the embedded Go code is enabled, send request to it instead of going through regular Python logic.
if self.config.go_feature_retrieval and self._go_server:
self._lazy_init_go_server()

entity_native_values: Dict[str, List[Any]]
if not native_entity_values:
# Convert proto types to native types since Go feature server currently
# only handles native types.
# TODO(felixwang9817): Remove this logic once native types are supported.
entity_native_values = {
k: [
feast_value_type_to_python_type(proto_value)
for proto_value in v
]
for k, v in entity_value_lists.items()
}
else:
entity_native_values = entity_value_lists

return self._go_server.get_online_features(
features_refs=features if isinstance(features, list) else [],
feature_service=features
if isinstance(features, FeatureService)
else None,
entities=entity_native_values,
request_data={}, # TODO: add request data parameter to public API
full_feature_names=full_feature_names,
)

_feature_refs = self._get_features(features, allow_cache=True)
(
requested_feature_views,
Expand Down Expand Up @@ -2283,45 +2226,12 @@ def serve(
) -> None:
"""Start the feature consumption server locally on a given port."""
type_ = type_.lower()
if self.config.go_feature_serving and self._go_server:
# Start go server instead of python if the flag is enabled
self._lazy_init_go_server()
enable_logging = (
self.config.feature_server
and self.config.feature_server.feature_logging
and self.config.feature_server.feature_logging.enabled
and not no_feature_log
)
logging_options = (
self.config.feature_server.feature_logging
if enable_logging and self.config.feature_server
else None
if type_ != "http":
raise ValueError(
f"Python server only supports 'http'. Got '{type_}' instead."
)
if type_ == "http":
self._go_server.start_http_server(
host,
port,
enable_logging=enable_logging,
logging_options=logging_options,
)
elif type_ == "grpc":
self._go_server.start_grpc_server(
host,
port,
enable_logging=enable_logging,
logging_options=logging_options,
)
else:
raise ValueError(
f"Unsupported server type '{type_}'. Must be one of 'http' or 'grpc'."
)
else:
if type_ != "http":
raise ValueError(
f"Python server only supports 'http'. Got '{type_}' instead."
)
# Start the python server if go server isn't enabled
feature_server.start_server(self, host, port, no_access_log)
# Start the python server
feature_server.start_server(self, host, port, no_access_log)

@log_exceptions_and_usage
def get_feature_server_endpoint(self) -> Optional[str]:
Expand Down Expand Up @@ -2367,9 +2277,6 @@ def serve_transformations(self, port: int) -> None:

transformation_server.start_server(self, port)

def _teardown_go_server(self):
self._go_server = None

@log_exceptions_and_usage
def write_logged_features(
self, logs: Union[pa.Table, Path], source: FeatureService
Expand Down
6 changes: 0 additions & 6 deletions sdk/python/feast/repo_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,12 +166,6 @@ class RepoConfig(FeastBaseModel):

repo_path: Optional[Path] = None

go_feature_serving: Optional[bool] = False
""" If True, use the Go feature server instead of the Python feature server. """

go_feature_retrieval: Optional[bool] = False
""" If True, use the embedded Go code to retrieve features instead of the Python SDK. """

entity_key_serialization_version: StrictInt = 1
""" Entity key serialization version: This version is used to control what serialization scheme is
used when writing data to the online store.
Expand Down
28 changes: 0 additions & 28 deletions sdk/python/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,6 @@ def pytest_configure(config):
"markers", "integration: mark test that has external dependencies"
)
config.addinivalue_line("markers", "benchmark: mark benchmarking tests")
config.addinivalue_line(
"markers", "goserver: mark tests that use the go feature server"
)
config.addinivalue_line(
"markers",
"universal_online_stores: mark tests that can be run against different online stores",
Expand All @@ -106,18 +103,11 @@ def pytest_addoption(parser):
default=False,
help="Run benchmark tests",
)
parser.addoption(
"--goserver",
action="store_true",
default=False,
help="Run tests that use the go feature server",
)


def pytest_collection_modifyitems(config, items: List[Item]):
should_run_integration = config.getoption("--integration") is True
should_run_benchmark = config.getoption("--benchmark") is True
should_run_goserver = config.getoption("--goserver") is True

integration_tests = [t for t in items if "integration" in t.keywords]
if not should_run_integration:
Expand All @@ -137,15 +127,6 @@ def pytest_collection_modifyitems(config, items: List[Item]):
for t in benchmark_tests:
items.append(t)

goserver_tests = [t for t in items if "goserver" in t.keywords]
if not should_run_goserver:
for t in goserver_tests:
items.remove(t)
else:
items.clear()
for t in goserver_tests:
items.append(t)


@pytest.fixture
def simple_dataset_1() -> pd.DataFrame:
Expand Down Expand Up @@ -276,9 +257,6 @@ def pytest_generate_tests(metafunc: pytest.Metafunc):
]
)

if "goserver" in markers:
extra_dimensions.append({"go_feature_serving": True})

configs = []
if offline_stores:
for provider, offline_store_creator in offline_stores:
Expand All @@ -291,12 +269,6 @@ def pytest_generate_tests(metafunc: pytest.Metafunc):
"online_store_creator": online_store_creator,
**dim,
}
# temporary Go works only with redis
if config.get("go_feature_serving") and (
not isinstance(online_store, dict)
or online_store["type"] != "redis"
):
continue

# aws lambda works only with dynamo
if (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ class IntegrationTestRepoConfig:
full_feature_names: bool = True
infer_features: bool = False
python_feature_server: bool = False
go_feature_serving: bool = False

def __repr__(self) -> str:
if not self.online_store_creator:
Expand All @@ -61,7 +60,6 @@ def __repr__(self) -> str:
f"{self.offline_store_creator.__name__.split('.')[-1].replace('DataSourceCreator', '')}",
online_store_type,
f"python_fs:{self.python_feature_server}",
f"go_fs:{self.go_feature_serving}",
]
)

Expand All @@ -77,6 +75,5 @@ def __eq__(self, other):
and self.online_store == other.online_store
and self.offline_store_creator == other.offline_store_creator
and self.online_store_creator == other.online_store_creator
and self.go_feature_serving == other.go_feature_serving
and self.python_feature_server == other.python_feature_server
)
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,6 @@ def construct_test_environment(
batch_engine=test_repo_config.batch_engine,
repo_path=repo_dir_name,
feature_server=feature_server,
go_feature_serving=test_repo_config.go_feature_serving,
entity_key_serialization_version=entity_key_serialization_version,
)

Expand Down
Loading

0 comments on commit 5e62844

Please sign in to comment.