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.

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

Python-centric feast deploy CLI #1362

Merged
merged 14 commits into from
Mar 10, 2021
65 changes: 0 additions & 65 deletions sdk/python/feast/infra/firestore.py

This file was deleted.

8 changes: 5 additions & 3 deletions sdk/python/feast/infra/local_sqlite.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,17 @@

from feast import FeatureTable
from feast.infra.provider import Provider
from feast.repo_config import LocalConfig
from feast.repo_config import LocalOnlineStoreConfig


def _table_id(project: str, table: FeatureTable) -> str:
return f"{project}_{table.name}"


class Sqlite(Provider):
def __init__(self, config: LocalConfig):
class LocalSqlite(Provider):
_db_path: str

def __init__(self, config: LocalOnlineStoreConfig):
self._db_path = config.path

def update_infra(
Expand Down
17 changes: 9 additions & 8 deletions sdk/python/feast/infra/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from typing import List

from feast import FeatureTable
from feast.repo_config import FirestoreConfig, OnlineStoreConfig
from feast.repo_config import RepoConfig


class Provider(abc.ABC):
woop marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -35,14 +35,15 @@ def teardown_infra(self, project: str, tables: List[FeatureTable]):
...


def get_provider(config: OnlineStoreConfig) -> Provider:
if config.type == "firestore":
from feast.infra.firestore import Firestore
def get_provider(config: RepoConfig) -> Provider:
if config.provider == "gcp":
from feast.infra.gcp import Gcp

return Firestore()
elif config.type == "local":
from feast.infra.local_sqlite import Sqlite
return Gcp()
elif config.provider == "local":
from feast.infra.local_sqlite import LocalSqlite

return Sqlite(config.local)
assert config.online_store.local is not None
woop marked this conversation as resolved.
Show resolved Hide resolved
return LocalSqlite(config.online_store.local)
else:
raise ValueError(config)
12 changes: 3 additions & 9 deletions sdk/python/feast/repo_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,12 @@
from bindr import bind


class FirestoreConfig(NamedTuple):
dummy: Optional[str] = None


class LocalConfig(NamedTuple):
path: Optional[str] = None
class LocalOnlineStoreConfig(NamedTuple):
path: str


class OnlineStoreConfig(NamedTuple):
type: str
firestore: Optional[FirestoreConfig] = None
local: Optional[LocalConfig] = None
local: Optional[LocalOnlineStoreConfig] = None


class RepoConfig(NamedTuple):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we just called this Config?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we already have a Config or two so I wanted to be a bit more specific here

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should reconcile https://github.com/feast-dev/feast/blob/master/sdk/python/feast/feature_store_config.py and this since I think they're the same thing

Expand Down
4 changes: 2 additions & 2 deletions sdk/python/feast/repo_operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def apply_total(repo_config: RepoConfig, repo_path: Path):
for table in repo.feature_tables:
registry.apply_feature_table(table, project)

infra_provider = get_provider(repo_config.online_store)
infra_provider = get_provider(repo_config)
infra_provider.update_infra(
project, tables_to_delete=tables_to_delete, tables_to_keep=repo.feature_tables
)
Expand All @@ -82,7 +82,7 @@ def teardown(repo_config: RepoConfig, repo_path: Path):
registry = Registry(repo_config.metadata_store)
project = repo_config.project
registry_tables = registry.list_feature_tables(project=project)
infra_provider = get_provider(repo_config.online_store)
infra_provider = get_provider(repo_config)
infra_provider.teardown_infra(project, tables=registry_tables)


Expand Down
55 changes: 53 additions & 2 deletions sdk/python/tests/cli/test_cli_local.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,54 @@
import subprocess
import sys
import tempfile
from pathlib import Path
from textwrap import dedent
from typing import List

from feast import cli


class CliRunner:
"""
NB. We can't use test runner helper from click here, since it doesn't start a new Python
interpreter. And we need a new interpreter for each test since we dynamically import
modules from the feature repo, and it is hard to clean up that state otherwise.
"""

def run(self, args: List[str], cwd: Path) -> subprocess.CompletedProcess:
woop marked this conversation as resolved.
Show resolved Hide resolved
return subprocess.run([sys.executable, cli.__file__] + args, cwd=cwd)


class TestCliLocal:
def test_hello(self):
pass
def test_hello(self) -> None:
runner = CliRunner()
with tempfile.TemporaryDirectory() as repo_dir_name, tempfile.TemporaryDirectory() as data_dir_name:

repo_path = Path(repo_dir_name)
data_path = Path(data_dir_name)

repo_config = repo_path / "feature_store.yaml"

repo_config.write_text(
dedent(
f"""
project: foo
metadata_store: {data_path / "metadata.db"}
provider: gcp
online_store:
local:
path: {data_path / "online_store.db"}
"""
)
)

repo_example = repo_path / "example.py"
repo_example.write_text(
(Path(__file__).parent / "example_feature_repo_1.py").read_text()
)

result = runner.run(["apply", str(repo_path)], cwd=repo_path)
assert result.returncode == 0

result = runner.run(["teardown", str(repo_path)], cwd=repo_path)
assert result.returncode == 0