From e17717b5c977348c619e0c48988c3f68ecc096aa Mon Sep 17 00:00:00 2001 From: Rishabh <158596025+RishabhCodezZz@users.noreply.github.com> Date: Wed, 19 Aug 2026 19:36:51 +0530 Subject: [PATCH 1/3] test: Add failing regression test for remote online store provisioning Applying a new feature view through a client whose online store is `type: remote` leaves it unwritable: the feature view reaches the registry, but no table is provisioned server-side, so the write that `feast materialize` performs returns HTTP 500 with `no such table`. The existing test_remote_online_store_read_write does apply a feature view through a remote client and write to it, but it applies `driver_hourly_stats` -- a name the server already provisioned during its own `feast apply` at setup. The table exists before the client touches it, which masks the bug. This test applies a feature view the server has never seen. The assertion is on the user-visible contract (a feature view applied through a remote online store must be writable) rather than on any particular mechanism, so it holds regardless of how the fix is implemented. Refs #6693 Signed-off-by: Rishabh <158596025+RishabhCodezZz@users.noreply.github.com> --- .../test_remote_online_store_provisioning.py | 217 ++++++++++++++++++ 1 file changed, 217 insertions(+) create mode 100644 sdk/python/tests/integration/online_store/test_remote_online_store_provisioning.py diff --git a/sdk/python/tests/integration/online_store/test_remote_online_store_provisioning.py b/sdk/python/tests/integration/online_store/test_remote_online_store_provisioning.py new file mode 100644 index 00000000000..7ead4a8b33d --- /dev/null +++ b/sdk/python/tests/integration/online_store/test_remote_online_store_provisioning.py @@ -0,0 +1,217 @@ +""" +Regression test for feast-dev/feast#6693. + + 'feast apply' does not create online store tables in remote mode. + +Contract under test (holds regardless of how the fix is implemented): + + Applying a NEW feature view through a client whose online store is + `type: remote` must leave that feature view writable. Today it does not: + the feature view lands in the registry, but no table is ever provisioned + server-side, so the write that `feast materialize` performs returns 500. + +Note on the existing coverage: `test_remote_online_store_read_write` in +sdk/python/tests/integration/online_store/test_remote_online_store.py already +applies a feature view through a remote client and writes to it, and passes. +It passes because it applies `driver_hourly_stats` -- a name the server had +already provisioned during its own `feast apply` at setup. The table exists +before the client ever touches it, which masks the bug. This test applies a +feature view the server has never seen. +""" + +import os +import socket +import subprocess +import tempfile +import time +from datetime import timedelta +from textwrap import dedent + +import pandas as pd +import pytest + +from feast import Entity, FeatureStore, FeatureView, FileSource +from feast.driver_test_data import create_driver_hourly_stats_df +from feast.field import Field +from feast.types import Float32, Int64 +from feast.utils import _utc_now + +FEAST_BIN = "feast" +PROJECT = "remote_provisioning" + +# A feature view name the server has never applied. This is the whole point. +NEW_FV_NAME = "never_applied_server_side" + + +def _free_port() -> int: + s = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + s.bind(("", 0)) + port = s.getsockname()[1] + s.close() + return port + + +def _port_open(host: str, port: int) -> bool: + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: + s.settimeout(1.0) + return s.connect_ex((host, port)) == 0 + + +@pytest.fixture(scope="module") +def remote_server(): + """`feast init` + `feast apply` + `feast serve`, backed by SQLite.""" + tmp = tempfile.mkdtemp(prefix="feast_6693_server_") + + subprocess.run( + [FEAST_BIN, "init", PROJECT], cwd=tmp, capture_output=True, check=True + ) + repo_path = os.path.join(tmp, PROJECT, "feature_repo") + subprocess.run( + [FEAST_BIN, "-c", repo_path, "apply"], cwd=tmp, capture_output=True, check=True + ) + + registry_path = os.path.join(repo_path, "data", "registry.db") + assert os.path.exists(registry_path), "server registry was not created" + + port = _free_port() + # Log to files, NOT subprocess.PIPE: `feast serve` is chatty, and an + # undrained pipe fills its 64K OS buffer and deadlocks the server. + log_path = os.path.join(tmp, "server.log") + log_file = open(log_path, "w") + proc = subprocess.Popen( + [ + FEAST_BIN, + "-c", + repo_path, + "serve", + "--host", + "127.0.0.1", + "--port", + str(port), + ], + stdout=log_file, + stderr=subprocess.STDOUT, + text=True, + ) + + def _server_log() -> str: + log_file.flush() + with open(log_path) as f: + return f.read() + + deadline = time.time() + 90 + while time.time() < deadline: + if proc.poll() is not None: + raise RuntimeError(f"feast serve exited early:\n{_server_log()}") + if _port_open("127.0.0.1", port): + break + time.sleep(1) + else: + proc.kill() + raise RuntimeError(f"feast serve did not come up in 90s:\n{_server_log()}") + + yield { + "url": f"http://127.0.0.1:{port}", + "registry_path": registry_path, + "server_log": _server_log, + } + + proc.kill() + proc.wait(timeout=30) + log_file.close() + + +@pytest.fixture(scope="module") +def client_store(remote_server) -> FeatureStore: + """A client whose online store is the remote feature server.""" + tmp = tempfile.mkdtemp(prefix="feast_6693_client_") + with open(os.path.join(tmp, "feature_store.yaml"), "w") as f: + f.write( + dedent( + f""" + project: {PROJECT} + registry: {remote_server["registry_path"]} + provider: local + entity_key_serialization_version: 3 + auth: + type: no_auth + online_store: + type: remote + path: {remote_server["url"]} + """ + ).strip() + ) + return FeatureStore(repo_path=tmp) + + +def _new_feature_view(data_path: str) -> tuple: + driver = Entity(name="driver_id", description="Driver id") + source = FileSource( + path=data_path, + timestamp_field="event_timestamp", + created_timestamp_column="created", + ) + fv = FeatureView( + name=NEW_FV_NAME, + entities=[driver], + ttl=timedelta(days=1), + schema=[ + Field(name="conv_rate", dtype=Float32), + Field(name="avg_daily_trips", dtype=Int64), + ], + source=source, + ) + return driver, source, fv + + +@pytest.mark.integration +def test_apply_through_remote_online_store_provisions_the_table( + client_store, remote_server +): + """A feature view applied through a remote online store must be writable. + + Fails before the fix with HTTP 500 / `no such table`, because + RemoteOnlineStore.update() is a no-op and nothing provisions server-side. + """ + # A real parquet file, so nothing fails for want of a source on disk. + data_dir = os.path.join(client_store.repo_path, "data") + os.makedirs(data_dir, exist_ok=True) + data_path = os.path.join(data_dir, "driver_stats.parquet") + end = _utc_now().replace(microsecond=0, second=0, minute=0) + create_driver_hourly_stats_df([1001], end - timedelta(days=1), end).to_parquet( + path=data_path, allow_truncated_timestamps=True + ) + + driver, source, fv = _new_feature_view(data_path) + client_store.apply([driver, fv]) + + # The registry accepted it -- the feature view genuinely exists. + assert NEW_FV_NAME in [v.name for v in client_store.list_feature_views()] + + now = pd.Timestamp(_utc_now()).round("ms") + df = pd.DataFrame( + { + "driver_id": [1001], + "conv_rate": [0.75], + "avg_daily_trips": [42], + "event_timestamp": [now], + "created": [now], + } + ) + + # This is what `feast materialize` does through the feature server. + try: + client_store.write_to_online_store(feature_view_name=NEW_FV_NAME, df=df) + except Exception: + print("\n------------------- server log (tail) -------------------") + print(remote_server["server_log"]()[-4000:]) + print("---------------------------------------------------------") + raise + + features = client_store.get_online_features( + features=[f"{NEW_FV_NAME}:conv_rate", f"{NEW_FV_NAME}:avg_daily_trips"], + entity_rows=[{"driver_id": 1001}], + ).to_dict() + + assert features["avg_daily_trips"] == [42] + assert round(features["conv_rate"][0], 2) == 0.75 From bd367e48d8f412d6ea65f2c04cd20d6e58ddfa2e Mon Sep 17 00:00:00 2001 From: Rishabh <158596025+RishabhCodezZz@users.noreply.github.com> Date: Wed, 19 Aug 2026 19:47:19 +0530 Subject: [PATCH 2/3] fix: Provision online store infra for remote feast apply `feast apply` against `online_store: type: remote` registered feature views but never created their tables, so `feast materialize` failed with HTTP 500 and `no such table`. `RemoteOnlineStore.update()` and `.teardown()` were both `pass`, and the feature server never calls `update_infra()` on its own -- it only refreshes the registry. Since table DDL lives exclusively in the concrete online store's `update()`, this affected any online store behind `type: remote`, not just SQLite. RemoteOnlineStore.update()/teardown() now call two new feature server endpoints, /update-infra and /teardown-infra, which run update_infra() and teardown_infra() against the server's real online store. The objects travel in the request body as base64-encoded protos rather than being looked up server-side by name, because FeatureStore.apply() calls update_infra() *before* registry.commit() -- at that point the server cannot see them yet, and with a file registry they are not even on disk. Authorization reuses the existing CREATE and DELETE actions, so no new permission surface is introduced. Fixes #6693 Signed-off-by: Rishabh <158596025+RishabhCodezZz@users.noreply.github.com> --- .../feature-servers/python-feature-server.md | 2 + sdk/python/feast/feature_server.py | 77 ++++++++++ .../feast/infra/online_stores/remote.py | 134 ++++++++++++++++- .../online_store/test_remote_online_store.py | 139 +++++++++++++++++- 4 files changed, 349 insertions(+), 3 deletions(-) diff --git a/docs/reference/feature-servers/python-feature-server.md b/docs/reference/feature-servers/python-feature-server.md index b1b873cc7d2..23d0d97e0e7 100644 --- a/docs/reference/feature-servers/python-feature-server.md +++ b/docs/reference/feature-servers/python-feature-server.md @@ -643,6 +643,8 @@ The [PyTorch NLP template](https://github.com/feast-dev/feast/tree/main/sdk/pyth | /write-to-online-store | FeatureView | Write Online | Write features to the online store | | /materialize | FeatureView | Write Online | Materialize features within a specified time range | | /materialize-incremental | FeatureView | Write Online | Incrementally materialize features up to a specified timestamp | +| /update-infra | FeatureView | Create, Delete | Provision online store infrastructure for a remote `feast apply` | +| /teardown-infra | FeatureView | Delete | Drop online store infrastructure for a remote `feast teardown` | ## How to configure Authentication and Authorization ? diff --git a/sdk/python/feast/feature_server.py b/sdk/python/feast/feature_server.py index e42e28d6db9..e7c7eda9bbc 100644 --- a/sdk/python/feast/feature_server.py +++ b/sdk/python/feast/feature_server.py @@ -83,6 +83,29 @@ class WriteToFeatureStoreRequest(BaseModel): transform_on_write: bool = True +class UpdateInfraRequest(BaseModel): + """Objects to provision, sent by RemoteOnlineStore.update(). + + Each entry is `{"type": , "proto": }`. They are + carried in the body rather than looked up in the registry because + `FeatureStore.apply()` runs `update_infra()` before `registry.commit()`, + so the server cannot see them yet. + """ + + tables_to_delete: List[Dict[str, str]] = [] + tables_to_keep: List[Dict[str, str]] = [] + entities_to_delete: List[Dict[str, str]] = [] + entities_to_keep: List[Dict[str, str]] = [] + partial: bool = False + + +class TeardownInfraRequest(BaseModel): + """Objects to tear down, sent by RemoteOnlineStore.teardown().""" + + tables: List[Dict[str, str]] = [] + entities: List[Dict[str, str]] = [] + + class PushFeaturesRequest(BaseModel): push_source_name: str df: dict @@ -932,6 +955,60 @@ async def write_to_online_store(request: WriteToFeatureStoreRequest) -> None: transform_on_write=request.transform_on_write, ) + @app.post("/update-infra", dependencies=[Depends(inject_user_details)]) + async def update_infra(request: UpdateInfraRequest) -> None: + """Provision online store infrastructure on behalf of a remote client. + + `feast apply` against `online_store: type: remote` cannot create tables + itself -- the concrete online store lives here. Without this the feature + view is registered but its table never exists (#6693). + """ + # Imported here, not at module scope: feast.infra.online_stores.remote + # imports from `feast`, which is still initializing when this module loads. + from feast.infra.online_stores.remote import decode_infra_object + + tables_to_keep = [decode_infra_object(o) for o in request.tables_to_keep] + tables_to_delete = [decode_infra_object(o) for o in request.tables_to_delete] + entities_to_keep = [decode_infra_object(o) for o in request.entities_to_keep] + entities_to_delete = [ + decode_infra_object(o) for o in request.entities_to_delete + ] + + for table in tables_to_keep: + assert_permissions(resource=table, actions=[AuthzedAction.CREATE]) + for table in tables_to_delete: + assert_permissions(resource=table, actions=[AuthzedAction.DELETE]) + + await run_in_threadpool( + store._get_provider().update_infra, + project=store.project, + tables_to_delete=tables_to_delete, + tables_to_keep=tables_to_keep, + entities_to_delete=entities_to_delete, + entities_to_keep=entities_to_keep, + partial=request.partial, + ) + + @app.post("/teardown-infra", dependencies=[Depends(inject_user_details)]) + async def teardown_infra(request: TeardownInfraRequest) -> None: + """Drop online store infrastructure on behalf of a remote client.""" + # Imported here, not at module scope: feast.infra.online_stores.remote + # imports from `feast`, which is still initializing when this module loads. + from feast.infra.online_stores.remote import decode_infra_object + + tables = [decode_infra_object(o) for o in request.tables] + entities = [decode_infra_object(o) for o in request.entities] + + for table in tables: + assert_permissions(resource=table, actions=[AuthzedAction.DELETE]) + + await run_in_threadpool( + store._get_provider().teardown_infra, + project=store.project, + tables=tables, + entities=entities, + ) + @app.get("/health") async def health(): try: diff --git a/sdk/python/feast/infra/online_stores/remote.py b/sdk/python/feast/infra/online_stores/remote.py index 8aa5df1563e..add4278a28a 100644 --- a/sdk/python/feast/infra/online_stores/remote.py +++ b/sdk/python/feast/infra/online_stores/remote.py @@ -11,6 +11,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +import base64 import json import logging import uuid as uuid_module @@ -60,6 +61,67 @@ logger = logging.getLogger(__name__) +def _infra_object_types() -> Dict[str, Tuple[Any, Any]]: + """Map type name -> (feast class, proto class) for objects sent to /update-infra. + + Imported lazily: these modules import from `feast` at module scope, and + pulling them in at the top of this file would create an import cycle. + """ + from feast.labeling.label_view import LabelView + from feast.on_demand_feature_view import OnDemandFeatureView + from feast.protos.feast.core.Entity_pb2 import Entity as EntityProto + from feast.protos.feast.core.FeatureView_pb2 import FeatureView as FeatureViewProto + from feast.protos.feast.core.LabelView_pb2 import LabelView as LabelViewProto + from feast.protos.feast.core.OnDemandFeatureView_pb2 import ( + OnDemandFeatureView as OnDemandFeatureViewProto, + ) + from feast.protos.feast.core.StreamFeatureView_pb2 import ( + StreamFeatureView as StreamFeatureViewProto, + ) + from feast.stream_feature_view import StreamFeatureView + + return { + "FeatureView": (FeatureView, FeatureViewProto), + "StreamFeatureView": (StreamFeatureView, StreamFeatureViewProto), + "OnDemandFeatureView": (OnDemandFeatureView, OnDemandFeatureViewProto), + "LabelView": (LabelView, LabelViewProto), + "Entity": (Entity, EntityProto), + } + + +def encode_infra_object(obj: Any) -> Dict[str, str]: + """Serialize a feature view or entity for transport to the feature server. + + The registry is not a usable channel here: `FeatureStore.apply()` calls + `update_infra()` *before* `registry.commit()`, so the server cannot yet see + these objects. They travel in the request body instead. + """ + type_name = type(obj).__name__ + if type_name not in _infra_object_types(): + raise ValueError( + f"Cannot send {type_name} to the remote online store; " + f"expected one of {sorted(_infra_object_types())}" + ) + return { + "type": type_name, + "proto": base64.b64encode(obj.to_proto().SerializeToString()).decode("ascii"), + } + + +def decode_infra_object(payload: Mapping[str, str]) -> Any: + """Inverse of :func:`encode_infra_object`, used by the feature server.""" + type_name = payload["type"] + types = _infra_object_types() + if type_name not in types: + raise ValueError( + f"Unknown object type {type_name!r}; expected one of {sorted(types)}" + ) + feast_class, proto_class = types[type_name] + return feast_class.from_proto( + proto_class.FromString(base64.b64decode(payload["proto"])) + ) + + def _json_safe(val: Any) -> Any: """Convert uuid.UUID objects and sets to JSON-serializable form.""" if isinstance(val, uuid_module.UUID): @@ -742,7 +804,33 @@ def update( entities_to_keep: Sequence[Entity], partial: bool, ): - pass + """Provision online store infrastructure through the feature server. + + Table creation happens inside the concrete online store's `update()`. + In remote mode that store lives server-side, so `feast apply` has to ask + the feature server to run it -- otherwise the feature view is registered + but its table never exists, and materialization fails with a missing + table (#6693). + """ + assert isinstance(config.online_store, RemoteOnlineStoreConfig) + config.online_store.__class__ = RemoteOnlineStoreConfig + + req_body = { + "tables_to_delete": [encode_infra_object(t) for t in tables_to_delete], + "tables_to_keep": [encode_infra_object(t) for t in tables_to_keep], + "entities_to_delete": [encode_infra_object(e) for e in entities_to_delete], + "entities_to_keep": [encode_infra_object(e) for e in entities_to_keep], + "partial": partial, + } + + response = post_remote_update_infra(config=config, req_body=req_body) + if response.status_code != 200: + error_msg = ( + "Unable to update online store infrastructure using feature server API. " + f"Error_code={response.status_code}, error_message={response.text}" + ) + logger.error(error_msg) + raise RuntimeError(error_msg) def teardown( self, @@ -750,7 +838,27 @@ def teardown( tables: Sequence[FeatureView], entities: Sequence[Entity], ): - pass + """Drop online store infrastructure through the feature server. + + The mirror of :meth:`update`: without this, `feast teardown` in remote + mode leaves every table behind. + """ + assert isinstance(config.online_store, RemoteOnlineStoreConfig) + config.online_store.__class__ = RemoteOnlineStoreConfig + + req_body = { + "tables": [encode_infra_object(t) for t in tables], + "entities": [encode_infra_object(e) for e in entities], + } + + response = post_remote_teardown_infra(config=config, req_body=req_body) + if response.status_code != 200: + error_msg = ( + "Unable to teardown online store infrastructure using feature server API. " + f"Error_code={response.status_code}, error_message={response.text}" + ) + logger.error(error_msg) + raise RuntimeError(error_msg) async def close(self) -> None: """ @@ -812,3 +920,25 @@ def post_remote_online_write( return session.post(url, json=req_body, verify=config.online_store.cert) else: return session.post(url, json=req_body) + + +@rest_error_handling_decorator +def post_remote_update_infra( + session: requests.Session, config: RepoConfig, req_body: dict +) -> requests.Response: + url = f"{config.online_store.path}/update-infra" + if config.online_store.cert: + return session.post(url, json=req_body, verify=config.online_store.cert) + else: + return session.post(url, json=req_body) + + +@rest_error_handling_decorator +def post_remote_teardown_infra( + session: requests.Session, config: RepoConfig, req_body: dict +) -> requests.Response: + url = f"{config.online_store.path}/teardown-infra" + if config.online_store.cert: + return session.post(url, json=req_body, verify=config.online_store.cert) + else: + return session.post(url, json=req_body) diff --git a/sdk/python/tests/unit/infra/online_store/test_remote_online_store.py b/sdk/python/tests/unit/infra/online_store/test_remote_online_store.py index 3babb384f6f..23c13549c48 100644 --- a/sdk/python/tests/unit/infra/online_store/test_remote_online_store.py +++ b/sdk/python/tests/unit/infra/online_store/test_remote_online_store.py @@ -8,7 +8,12 @@ from feast import Entity, FeatureView, Field, FileSource, RepoConfig from feast.feature_service import FeatureService from feast.infra.online_stores.online_store import OnlineStore -from feast.infra.online_stores.remote import RemoteOnlineStore, RemoteOnlineStoreConfig +from feast.infra.online_stores.remote import ( + RemoteOnlineStore, + RemoteOnlineStoreConfig, + decode_infra_object, + encode_infra_object, +) from feast.online_response import OnlineResponse from feast.protos.feast.types.EntityKey_pb2 import EntityKey as EntityKeyProto from feast.protos.feast.types.Value_pb2 import Value as ValueProto @@ -779,3 +784,135 @@ def test_unix_timestamp_value_serialized_as_int( # Event timestamps should be ISO strings as before assert df["event_timestamp"] == ["2023-11-15T00:00:00"] + + +class TestRemoteOnlineStoreUpdateInfra: + """Tests for RemoteOnlineStore.update / teardown. + + Both were no-ops, so `feast apply` and `feast teardown` in remote mode never + provisioned or dropped anything server-side (#6693). + """ + + @pytest.fixture + def remote_store(self): + return RemoteOnlineStore() + + @pytest.fixture + def config(self): + return RepoConfig( + project="test_project", + online_store=RemoteOnlineStoreConfig( + type="remote", path="http://localhost:6566" + ), + registry="dummy_registry", + ) + + @pytest.fixture + def entity(self): + return Entity(name="user_id", value_type=ValueType.INT64) + + @pytest.fixture + def feature_view(self, entity): + return FeatureView( + name="test_feature_view", + entities=[entity], + ttl=timedelta(days=1), + schema=[ + Field(name="user_id", dtype=Int64), + Field(name="feature1", dtype=String), + ], + source=FileSource(path="test.parquet", timestamp_field="event_timestamp"), + ) + + def test_infra_object_round_trip(self, feature_view, entity): + """Objects must survive the base64 proto hop to the feature server.""" + for original in (feature_view, entity): + decoded = decode_infra_object(encode_infra_object(original)) + assert type(decoded) is type(original) + assert decoded.name == original.name + + def test_encode_rejects_unsupported_type(self): + with pytest.raises(ValueError, match="Cannot send"): + encode_infra_object(object()) + + def test_decode_rejects_unknown_type(self): + with pytest.raises(ValueError, match="Unknown object type"): + decode_infra_object({"type": "NotAFeastObject", "proto": ""}) + + @patch("feast.infra.online_stores.remote.post_remote_update_infra") + def test_update_posts_tables_to_the_feature_server( + self, mock_post, remote_store, config, feature_view, entity + ): + """update() must send the objects, not silently do nothing.""" + mock_response = Mock() + mock_response.status_code = 200 + mock_post.return_value = mock_response + + remote_store.update( + config=config, + tables_to_delete=[], + tables_to_keep=[feature_view], + entities_to_delete=[], + entities_to_keep=[entity], + partial=False, + ) + + mock_post.assert_called_once() + req_body = mock_post.call_args[1]["req_body"] + + assert [t["type"] for t in req_body["tables_to_keep"]] == ["FeatureView"] + assert [e["type"] for e in req_body["entities_to_keep"]] == ["Entity"] + assert req_body["tables_to_delete"] == [] + assert req_body["partial"] is False + + # The payload must be decodable back into the same feature view. + assert ( + decode_infra_object(req_body["tables_to_keep"][0]).name == feature_view.name + ) + + @patch("feast.infra.online_stores.remote.post_remote_update_infra") + def test_update_raises_on_error_response( + self, mock_post, remote_store, config, feature_view + ): + """A failed provisioning call must not pass silently -- that is the bug.""" + mock_response = Mock() + mock_response.status_code = 500 + mock_response.text = "boom" + mock_post.return_value = mock_response + + with pytest.raises(RuntimeError, match="Unable to update online store"): + remote_store.update( + config=config, + tables_to_delete=[], + tables_to_keep=[feature_view], + entities_to_delete=[], + entities_to_keep=[], + partial=False, + ) + + @patch("feast.infra.online_stores.remote.post_remote_teardown_infra") + def test_teardown_posts_tables_to_the_feature_server( + self, mock_post, remote_store, config, feature_view, entity + ): + mock_response = Mock() + mock_response.status_code = 200 + mock_post.return_value = mock_response + + remote_store.teardown(config=config, tables=[feature_view], entities=[entity]) + + mock_post.assert_called_once() + req_body = mock_post.call_args[1]["req_body"] + assert [t["type"] for t in req_body["tables"]] == ["FeatureView"] + assert [e["type"] for e in req_body["entities"]] == ["Entity"] + + @patch("feast.infra.online_stores.remote.post_remote_teardown_infra") + def test_teardown_raises_on_error_response( + self, mock_post, remote_store, config, feature_view + ): + mock_response = Mock() + mock_response.status_code = 503 + mock_response.text = "unavailable" + mock_post.return_value = mock_response + + with pytest.raises(RuntimeError, match="Unable to teardown online store"): + remote_store.teardown(config=config, tables=[feature_view], entities=[]) From 70459ce8b7ef39cd05ab7ecea956dd7b1a6a9014 Mon Sep 17 00:00:00 2001 From: Rishabh <158596025+RishabhCodezZz@users.noreply.github.com> Date: Sat, 22 Aug 2026 22:16:47 +0530 Subject: [PATCH 3/3] fix: Authorize entities on infra endpoints and cover teardown end to end Review follow-ups on the remote online store provisioning endpoints. /update-infra and /teardown-infra asserted permissions on the feature views in the request body but not on the entities, so a caller could provision or drop entity infrastructure without an authorization check. Both endpoints now apply the same CREATE and DELETE actions to entities that they already apply to tables. _infra_object_types() re-ran its five lazy imports on every call, and encode_infra_object() invoked it twice per object across four lists. It is now cached and read once per call. The integration test proved that apply() provisions a table but never exercised the teardown path, and both of its fixtures leaked their temp directories. It now asserts against the server's own SQLite file on both sides of the lifecycle -- the table exists after apply and is gone after teardown, which covers /update-infra and /teardown-infra end to end -- and removes its temp directories on the way out. Co-Authored-By: Claude Opus 5 Signed-off-by: Rishabh <158596025+RishabhCodezZz@users.noreply.github.com> --- sdk/python/feast/feature_server.py | 6 ++ .../feast/infra/online_stores/remote.py | 9 ++- .../test_remote_online_store_provisioning.py | 58 +++++++++++++++++-- 3 files changed, 66 insertions(+), 7 deletions(-) diff --git a/sdk/python/feast/feature_server.py b/sdk/python/feast/feature_server.py index e7c7eda9bbc..d0ce1cb1e60 100644 --- a/sdk/python/feast/feature_server.py +++ b/sdk/python/feast/feature_server.py @@ -978,6 +978,10 @@ async def update_infra(request: UpdateInfraRequest) -> None: assert_permissions(resource=table, actions=[AuthzedAction.CREATE]) for table in tables_to_delete: assert_permissions(resource=table, actions=[AuthzedAction.DELETE]) + for entity in entities_to_keep: + assert_permissions(resource=entity, actions=[AuthzedAction.CREATE]) + for entity in entities_to_delete: + assert_permissions(resource=entity, actions=[AuthzedAction.DELETE]) await run_in_threadpool( store._get_provider().update_infra, @@ -1001,6 +1005,8 @@ async def teardown_infra(request: TeardownInfraRequest) -> None: for table in tables: assert_permissions(resource=table, actions=[AuthzedAction.DELETE]) + for entity in entities: + assert_permissions(resource=entity, actions=[AuthzedAction.DELETE]) await run_in_threadpool( store._get_provider().teardown_infra, diff --git a/sdk/python/feast/infra/online_stores/remote.py b/sdk/python/feast/infra/online_stores/remote.py index add4278a28a..4f6c0cc6fd9 100644 --- a/sdk/python/feast/infra/online_stores/remote.py +++ b/sdk/python/feast/infra/online_stores/remote.py @@ -17,6 +17,7 @@ import uuid as uuid_module from collections import defaultdict from datetime import datetime +from functools import lru_cache from typing import ( Any, Callable, @@ -61,11 +62,14 @@ logger = logging.getLogger(__name__) +@lru_cache(maxsize=1) def _infra_object_types() -> Dict[str, Tuple[Any, Any]]: """Map type name -> (feast class, proto class) for objects sent to /update-infra. Imported lazily: these modules import from `feast` at module scope, and pulling them in at the top of this file would create an import cycle. + Cached because `update()` calls this once per object in four lists, and + re-running the imports each time is pure overhead. """ from feast.labeling.label_view import LabelView from feast.on_demand_feature_view import OnDemandFeatureView @@ -96,11 +100,12 @@ def encode_infra_object(obj: Any) -> Dict[str, str]: `update_infra()` *before* `registry.commit()`, so the server cannot yet see these objects. They travel in the request body instead. """ + types = _infra_object_types() type_name = type(obj).__name__ - if type_name not in _infra_object_types(): + if type_name not in types: raise ValueError( f"Cannot send {type_name} to the remote online store; " - f"expected one of {sorted(_infra_object_types())}" + f"expected one of {sorted(types)}" ) return { "type": type_name, diff --git a/sdk/python/tests/integration/online_store/test_remote_online_store_provisioning.py b/sdk/python/tests/integration/online_store/test_remote_online_store_provisioning.py index 7ead4a8b33d..8f3af37a525 100644 --- a/sdk/python/tests/integration/online_store/test_remote_online_store_provisioning.py +++ b/sdk/python/tests/integration/online_store/test_remote_online_store_provisioning.py @@ -20,12 +20,15 @@ """ import os +import shutil import socket +import sqlite3 import subprocess import tempfile import time from datetime import timedelta from textwrap import dedent +from typing import Iterator, List import pandas as pd import pytest @@ -57,6 +60,27 @@ def _port_open(host: str, port: int) -> bool: return s.connect_ex((host, port)) == 0 +def _server_online_tables(repo_path: str) -> List[str]: + """Table names in the server's own SQLite online store. + + Read directly rather than through any Feast API: the point of this test is + that the table physically exists server-side, so asking Feast about it + would beg the question. Opened read-only -- the server process has the + file open too. + """ + db_path = os.path.join(repo_path, "data", "online_store.db") + # SqliteOnlineStore.teardown() unlinks the whole file rather than dropping + # tables one by one, so a missing file means "nothing left", not an error. + if not os.path.exists(db_path): + return [] + con = sqlite3.connect(f"file:{db_path}?mode=ro", uri=True) + try: + rows = con.execute("SELECT name FROM sqlite_master WHERE type='table'") + return [r[0] for r in rows] + finally: + con.close() + + @pytest.fixture(scope="module") def remote_server(): """`feast init` + `feast apply` + `feast serve`, backed by SQLite.""" @@ -113,16 +137,20 @@ def _server_log() -> str: yield { "url": f"http://127.0.0.1:{port}", "registry_path": registry_path, + "repo_path": repo_path, "server_log": _server_log, } proc.kill() proc.wait(timeout=30) log_file.close() + # ignore_errors: on Windows the just-killed server can still hold a handle + # to the SQLite files for a moment. + shutil.rmtree(tmp, ignore_errors=True) @pytest.fixture(scope="module") -def client_store(remote_server) -> FeatureStore: +def client_store(remote_server) -> Iterator[FeatureStore]: """A client whose online store is the remote feature server.""" tmp = tempfile.mkdtemp(prefix="feast_6693_client_") with open(os.path.join(tmp, "feature_store.yaml"), "w") as f: @@ -141,7 +169,9 @@ def client_store(remote_server) -> FeatureStore: """ ).strip() ) - return FeatureStore(repo_path=tmp) + yield FeatureStore(repo_path=tmp) + + shutil.rmtree(tmp, ignore_errors=True) def _new_feature_view(data_path: str) -> tuple: @@ -165,13 +195,16 @@ def _new_feature_view(data_path: str) -> tuple: @pytest.mark.integration -def test_apply_through_remote_online_store_provisions_the_table( +def test_remote_apply_provisions_the_table_and_teardown_drops_it( client_store, remote_server ): - """A feature view applied through a remote online store must be writable. + """The full remote lifecycle: apply provisions the table, teardown drops it. - Fails before the fix with HTTP 500 / `no such table`, because + Apply fails before the fix with HTTP 500 / `no such table`, because RemoteOnlineStore.update() is a no-op and nothing provisions server-side. + Teardown is the mirror: without RemoteOnlineStore.teardown() the table is + left behind. Both halves are asserted against the server's own SQLite file, + so this covers /update-infra and /teardown-infra end to end. """ # A real parquet file, so nothing fails for want of a source on disk. data_dir = os.path.join(client_store.repo_path, "data") @@ -215,3 +248,18 @@ def test_apply_through_remote_online_store_provisions_the_table( assert features["avg_daily_trips"] == [42] assert round(features["conv_rate"][0], 2) == 0.75 + + # The table is physically there in the server's store, not just implied by + # the write having succeeded. + tables = _server_online_tables(remote_server["repo_path"]) + assert any(NEW_FV_NAME in t for t in tables), ( + f"{NEW_FV_NAME} was never provisioned server-side; tables={tables}" + ) + + # And the mirror image: teardown must drop it again. + client_store.teardown() + + tables = _server_online_tables(remote_server["repo_path"]) + assert not any(NEW_FV_NAME in t for t in tables), ( + f"{NEW_FV_NAME} survived teardown; tables={tables}" + )