| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
| self.materialization_intervals.append((start_date, end_date)) | ||
| return self._cap_materialization_intervals(max_intervals) | ||
|
|
||
| def _cap_materialization_intervals( |
There was a problem hiding this comment.
I think _cap_materialization_intervals silently disables capping when max_intervals == 0. Is that intended?
Sorry, something went wrong.
There was a problem hiding this comment.
That was leftover from a previous iteration, removed.
Sorry, something went wrong.
| ) | ||
|
|
||
| Index( | ||
| "idx_materialization_interval_history_fv_project", |
There was a problem hiding this comment.
Looks like two concurrent calls (e.g. a retry racing the original) can both pass the check before either commits, producing duplicate history rows. Could you please check this?
Sorry, something went wrong.
There was a problem hiding this comment.
Added some checks to avoid this.
Sorry, something went wrong.
…kend
Adds a full, uncapped materialization-interval history, retained in the
registry itself (not delegated to an external system), compatible with
every registry backend -- file-based, SQL, Snowflake, HTTP, and gRPC --
plus a configurable cap on the existing rolling-window
FeatureView.materialization_intervals list.
Cap configurability:
- RegistryConfig.materialization_intervals_max_len (default 10, matching
the prior hardcoded MATERIALIZATION_INTERVALS_MAX_LEN constant) lets
deployments tune the cap via config instead of a code change -- e.g.
start permissive and lower it in steps. Wired into
SqlRegistry.apply_materialization and its _apply_object hydration path.
Durable interval history:
- New MaterializationIntervalHistoryEntry proto (protos/feast/core/FeatureView.proto)
and a new top-level Registry.materialization_interval_history field for
the file-based backend's "separate node" (protos/feast/core/Registry.proto).
- FeatureView.add_materialization_interval/update_materialization_intervals/
_cap_materialization_intervals now return the dropped intervals instead of
None, so callers can archive them -- purely additive, existing callers
ignoring the return value are unaffected.
- New BaseRegistry.get_materialization_interval_history abstract method,
implemented for real by every concrete backend (not duck-typed/backend-gated):
- SqlRegistry/SqlFallbackRegistry: new materialization_interval_history
table (auto-created via the existing metadata.create_all(), inherited
by SqlFallbackRegistry for free), idempotent inserts keyed on
(feature_view_name, project, start_time, end_time) so archiving both a
newly-added interval and, separately, a later-dropped one never
duplicates a row.
- File-based Registry: same idempotent semantics against the new
top-level proto field.
- SnowflakeRegistry: new MATERIALIZATION_INTERVAL_HISTORY table via
snowflake_table_creation.sql, bespoke inserts (bypassing the
upsert-by-key _apply_object/_get_object helpers, which don't fit a
genuinely multi-row-per-key table).
- RemoteRegistry/gRPC: new GetMaterializationIntervalHistory RPC
(RegistryServer.proto + registry_server.py handler + client method),
following the ListEntities pagination template. The existing
ApplyMaterialization RPC needed zero changes -- it already delegates
straight to the proxied registry's apply_materialization, so once every
backend archives internally, gRPC callers get correct behavior for free.
- HttpRegistry: now calls a dedicated endpoint instead of the old
client-side-append + whole-object PUT (see below).
Also fixes two real, pre-existing bugs surfaced while designing this:
1. File-based Registry and SnowflakeRegistry never enforced
materialization_intervals_max_len at all (silent divergence from
SqlRegistry) -- both now route through add_materialization_interval/
update_materialization_intervals like every other backend.
2. HttpRegistry.apply_materialization appended the new interval
client-side and PUT the whole FeatureView object to the generic
feature_views endpoint, bypassing the server's cap/archive logic
entirely and risking a silent clobber of previously-stored intervals if
the in-memory object wasn't fully hydrated. It now calls a dedicated
materialization endpoint (paired with a matching addition in
eg-feature-store-registry) so the server fetches the canonical stored
feature view itself.
New tests across feature_view.py, SqlRegistry, the file-based Registry,
SnowflakeRegistry (mocked -- no live Snowflake available), and the new
gRPC RPC/RemoteRegistry client. Full sdk/python/tests/unit suite: 1183
passed, 22 skipped, the same 9 pre-existing/unrelated failures as before
(sqlite disk I/O + one docling error), no new failures.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Unit tests are failing. Rest looks good to me.
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Adds a full, uncapped materialization-interval history, retained in the registry itself (not delegated to an external system), compatible with every registry backend -- file-based, SQL, Snowflake, HTTP, and gRPC -- plus a configurable cap on the existing rolling-window FeatureView.materialization_intervals list.
Cap configurability:
Durable interval history:
Also fixes two real, pre-existing bugs surfaced while designing this:
New tests across feature_view.py, SqlRegistry, the file-based Registry, SnowflakeRegistry (mocked -- no live Snowflake available), and the new gRPC RPC/RemoteRegistry client. Full sdk/python/tests/unit suite: 1183 passed, 22 skipped, the same 9 pre-existing/unrelated failures as before (sqlite disk I/O + one docling error), no new failures.
What this PR does / why we need it:
Which issue(s) this PR fixes:
Misc