fix: Feast apply silently ignoring ttl updates to None or timedelta(0) by saket3395 · Pull Request #6709 · feast-dev/feast · GitHub

Summary

Fixes #6703 — re-applying an existing FeatureView/LabelView with ttl=None or ttl=timedelta(0) (both the documented way to express "no ttl") silently keeps the old finite ttl in the registry. feast apply reports the change on every run, but nothing actually updates.

Root cause (as diagnosed in the issue): Registry._update_metadata_fields() routes ttl changes through a truthiness check:

if (
    hasattr(existing_proto.spec, "ttl")
    and hasattr(updated_fv, "ttl")
    and updated_fv.ttl          # falsy for both None and timedelta(0)
):

Since None and timedelta(0) are both falsy, the whole ttl-update block is skipped exactly when it should clear the ttl.

There's a second bug in the same block that the outer-gate fix alone doesn't cover: for the FeatureView branch, get_ttl_duration() returns Python None when self.ttl is None:

def get_ttl_duration(self):
    ttl_duration = None
    if self.ttl is not None:
        ttl_duration = Duration()
        ttl_duration.FromTimedelta(self.ttl)
    return ttl_duration

and the existing inner check in _update_metadata_fields:

ttl_duration = updated_fv.get_ttl_duration()
if ttl_duration:
    existing_proto.spec.ttl.CopyFrom(ttl_duration)

would still skip CopyFrom when ttl_duration is None — so removing only the outer gate fixes the timedelta(0) case but not ttl=None for FeatureView.

Fix

Removed the outer truthy gate; kept only the hasattr checks.
For FeatureView: always CopyFrom — using an explicit empty Duration() when get_ttl_duration() returns None, instead of skipping the write. An empty Duration decodes back to timedelta(0) via FeatureView.from_proto's existing ToNanoseconds() == 0 check, so this correctly round-trips as "no ttl".
For LabelView: guarded FromTimedelta() against ttl is None (previously unreachable because the outer gate prevented ttl=None from reaching this branch at all; now that the gate is gone, it needs its own guard) — falls through to the default zero Duration() in that case.

Verification

Traced all three cases (None, timedelta(0), a finite value) through the new branch logic in isolation (a minimal stand-in mirroring the real Duration/get_ttl_duration control flow, since protobuf isn't installed in this environment and I didn't want to add it just to verify branch logic):

ttl input	FeatureView branch	LabelView branch
None	zero Duration	zero Duration
timedelta(0)	zero Duration	zero Duration
timedelta(days=10)	864000s Duration	864000s Duration

Both branches now resolve identically for all three cases, and confirmed via feature_view.py's from_proto that a zero-value Duration round-trips to timedelta(0), matching the codebase's existing "no ttl" convention.

Test plan

Traced all three ttl cases through the new logic in isolation, for both FeatureView and LabelView.
Didn't run this against feast's actual test suite / a live feast apply round-trip in this environment (protobuf not installed here, didn't want to pull in the dependency just to verify this). Flagging for CI/maintainer verification — happy to iterate if anything surfaces.

LGTM, some nitpicks.

[Suggestion] Consider moving import to top of file

The Duration import is now used in both branches of the conditional. Moving it to the top of the file would be more conventional and slightly more efficient.

Suggested:

Suggested change

      
                        from google.protobuf.duration_pb2 import Duration
          
            # Move this import to the top of the file with other imports
          
            from google.protobuf.duration_pb2 import Duration

[Suggestion] Consider error handling for invalid timedelta

FromTimedelta could potentially raise an exception for invalid timedelta values. Consider adding basic validation or error handling to provide better user feedback.

Suggested:

Suggested change

      
                            ttl_duration = Duration()
          
                            ttl_duration.FromTimedelta(updated_fv.ttl)
          
                            if updated_fv.ttl is not None:
          
            ttl_duration = Duration()
          
            if updated_fv.ttl is not None:
          
                try:
          
                    ttl_duration.FromTimedelta(updated_fv.ttl)
          
                except (ValueError, OverflowError) as e:
          
                    raise ValueError(f"Invalid TTL value: {updated_fv.ttl}") from e

Thanks for the review, @jyejare! Addressed both: moved the Duration import to the top of the file with the other google.protobuf imports, and wrapped FromTimedelta() in a try/except that re-raises as a ValueError naming the offending value.

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 25.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 46.80%. Comparing base (a9219d9) to head (7dcb610).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
sdk/python/feast/infra/registry/registry.py	25.00%	5 Missing and 1 partial ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6709      +/-   ##
==========================================
- Coverage   46.80%   46.80%   -0.01%     
==========================================
  Files         415      415              
  Lines       50395    50398       +3     
  Branches     7214     7214              
==========================================
  Hits        23588    23588              
- Misses      25155    25158       +3     
  Partials     1652     1652

Flag	Coverage Δ
go-feature-server	30.58% <ø> (ø)
python-unit	48.13% <25.00%> (-0.01%)	⬇️

Files with missing lines	Coverage Δ
sdk/python/feast/infra/registry/registry.py	62.00% <25.00%> (-0.11%)	⬇️

... and 1 file with indirect coverage changes

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a9219d9...7dcb610. Read the comment docs.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@saket3395 can we have test coverage for this ?

@ntkathole added unit coverage in sdk/python/tests/unit/infra/registry/test_update_metadata_fields.py:

clearing a finite ttl to None or timedelta(0) now writes a zero Duration (parametrized over both) — this is the regression that was silently dropped before
a finite→finite ttl update is preserved

Since _update_metadata_fields uses no instance state, the test exercises it directly via the class, mirroring the FeatureView/FileSource/Field construction used elsewhere in the unit tests. I wasn't able to run the suite in my local environment, so I'd appreciate CI confirming it — happy to adjust if anything needs tweaking.

@saket3395 Please sign the commit and fix the linting checks

@ntkathole done — all commits are now DCO signed-off (DCO check is green), and I fixed the ruff format issue (the multi-line raise ValueError in the ttl block collapses to a single line under the 88-char limit). Ran ruff check and ruff format --check locally on both the changed source and the new test file — all clean now. Let me know if anything else is needed.

Fixed the lint-pr failure too — the "Validate PR title" job wanted a conventional-commit type, so I changed the title from Fix: to lowercase fix: (commitlint flagged type-case + type-enum). The re-run is showing action_required (pending maintainer approval to run on this fork PR) — should go green once it's approved or re-triggered.

…a(0) Fixes feast-dev#6703. Registry._update_metadata_fields() routes ttl changes on re-apply through a truthiness check: if (... and updated_fv.ttl): None and timedelta(0) are both the documented way to express "no ttl", and both are falsy, so re-applying a FeatureView/LabelView with ttl cleared silently kept the old finite ttl -- feast apply reported the update but nothing changed in the registry. Removing that outer truthy gate isn't sufficient by itself: for the FeatureView branch, get_ttl_duration() returns Python None when self.ttl is None, and the existing inner check if ttl_duration: existing_proto.spec.ttl.CopyFrom(ttl_duration) would still silently skip CopyFrom in that case, leaving the stale ttl in place. Fixed both by explicitly writing an empty Duration() (which decodes back to timedelta(0), per FeatureView.from_proto's existing ToNanoseconds()==0 check) whenever there's no real ttl to write, instead of skipping the write. The LabelView branch is adjusted the same way, guarding FromTimedelta() against a None ttl now that the outer gate no longer prevents ttl=None from reaching this branch. Traced all three cases (None, timedelta(0), a finite value) through the new branch logic in isolation and confirmed FeatureView and LabelView now resolve identically: None and timedelta(0) both produce a zero Duration, a finite ttl passes through unchanged. Signed-off-by: saket3395 <sakettulsan95@gmail.com>

- Move the Duration import to the top of the file with the other google.protobuf imports, consistent with how Message and RepeatedCompositeFieldContainer are already imported there. - Wrap FromTimedelta() in the LabelView branch with a try/except, re-raising as a ValueError naming the offending value, so an invalid timedelta surfaces a clear error instead of a raw protobuf exception. Signed-off-by: saket3395 <sakettulsan95@gmail.com>

Adds unit coverage requested in review: - clearing a finite ttl to None or timedelta(0) now writes a zero Duration (previously silently dropped) - a finite-to-finite ttl update is preserved _update_metadata_fields uses no instance state, so it is exercised directly via the class, mirroring the FeatureView/FileSource/Field construction used elsewhere in the unit tests. Signed-off-by: saket3395 <sakettulsan95@gmail.com>

Collapse the multi-line raise ValueError back to a single line per ruff format (it fits within the 88-char limit), fixing the format check. Signed-off-by: saket3395 <sakettulsan95@gmail.com>

# [0.66.0](v0.65.0...v0.66.0) (2026-08-21) ### Bug Fixes * Add connection pre-warming for DynamoDB async client ([89240fa](89240fa)), closes [#6060](#6060) * Add remote registry client extra ([#6697](#6697)) ([b8dfcb0](b8dfcb0)) * Address review feedback on FIPS cipher suite configuration ([4a35fba](4a35fba)) * Allow remote-registry first apply for new projects ([39d408d](39d408d)) * Avoid importing feast.feature_store at mcp_server import time ([ddb2e9a](ddb2e9a)) * Bump pymssql to >=2.3.6 for macOS arm64 wheel support ([181eb35](181eb35)), closes [#5636](#5636) [#5193](#5193) [#5636](#5636) * Call ApplySavedDataset RPC instead of ApplyFeatureService in RemoteRegistry.apply_saved_dataset() ([934d341](934d341)) * Catch missing dbt parser dependency in dbt CLI commands ([#6534](#6534)) ([3c2ae3c](3c2ae3c)) * Default authentication to kubernetes auth ([6a4690a](6a4690a)) * Defer feature-freshness thread to post-fork to avoid Gunicorn deadlock ([#6648](#6648)) ([104ad10](104ad10)), closes [#6647](#6647) * Do not pass undeclared feature view columns to ODFV UDFs ([#6527](#6527)) ([75b9463](75b9463)) * downgrade mcp pin to 1.29.0 and fix CI lockfiles and unit tests ([98e5bca](98e5bca)), closes [#6706](#6706) * Feast apply silently ignoring ttl updates to None or timedelta(0) ([#6709](#6709)) ([97b0f25](97b0f25)), closes [#6703](#6703) * Fix mypy TorchTensor type alias error ([#6712](#6712)) ([34de6fa](34de6fa)), closes [#5563](#5563) * Fixed data source creation form gaps ([5d0f7d6](5d0f7d6)) * Handle parameterized and complex Trino types in type map ([326554d](326554d)) * Isolate default user permissions ([e37adbf](e37adbf)) * Isolate projection join key maps ([d1c709d](d1c709d)) * Map Postgres real to FLOAT instead of DOUBLE ([62db435](62db435)) * Merge shared ODFV source projections in feature resolution ([d269946](d269946)), closes [#6621](#6621) * More exhaustive athena types ([a9aaefc](a9aaefc)) * Normalize SQL registry read_path to the psycopg3 driver like path ([#6644](#6644)) ([996c6ea](996c6ea)), closes [#6643](#6643) * **operator:** add spec.services.onlineStore.disabled to opt out of the online store ([d81d4e3](d81d4e3)), closes [#6586](#6586) * Preinstall DuckDB delta extension for tests ([fd4d49d](fd4d49d)), closes [#6743](#6743) * Preserve event-time ordering within Redis online_write_batch ([40fb788](40fb788)), closes [#5163](#5163) * Prevent mutation of cached feature resolution results ([ea17419](ea17419)) * Remote feastRef FeatureStore fails first apply for a new feastProject ([9affee5](9affee5)) * Remove inert subjectaccessreviews and reorganize RBAC rules ([f771ea4](f771ea4)) * Report single-feature-view spark_application materialization success ([a9219d9](a9219d9)), closes [#6673](#6673) * Reset the global security manager after the permissions fixture ([7667215](7667215)) * Resolve kserve with pip --dry-run instead of installing it ([01da132](01da132)), closes [#6732](#6732) * Resolve write_to_offline_store feature view with a single registry lookup ([a42dc85](a42dc85)), closes [#4235](#4235) * Return False from __eq__ on cross-type comparison ([#6637](#6637)) ([0f149a9](0f149a9)), closes [#6636](#6636) * Reuse IdP-issued client tokens until near expiry ([602d752](602d752)) * Reuse the OIDC JWKS client across requests ([#6683](#6683)) ([a1e6fc2](a1e6fc2)) * Separate CronJob and feature-server ServiceAccounts ([398f643](398f643)) * Serialize UnixTimestamp proto values as raw int64 in remote online store transport ([1e7134f](1e7134f)) * Set FIPS cipher suites before pyarrow.flight import to prevent crash on IBM Power ([979b82a](979b82a)) * Support Entra ID (Azure AD) token claims in OIDC auth ([#6631](#6631)) ([f843c63](f843c63)) * UDF/ODFV source rehydrate (+ Postgres / online cache) ([#6655](#6655)) ([5fd7af7](5fd7af7)) * Updated projects-list.json in order to display newly added projects ([#6657](#6657)) ([3a6a103](3a6a103)) * Use correct image name in multi-arch imagetools push step ([faf85e0](faf85e0)) * Use join keys instead of entity names in ODFV materialization ([#6645](#6645)) ([abffebc](abffebc)), closes [#5965](#5965) * use matching proto class per feature view list in SqliteOnlineStore.plan() ([adb8c1c](adb8c1c)), closes [#6658](#6658) * Widen Athena integer type mapping for unsigned ints ([3425783](3425783)) ### Features * Add ConnectionRef to DataSource for pluggable external credential resolution ([28bde01](28bde01)) * Add Feature Service Create in UI ([0399380](0399380)) * Add hybrid to ValidOfflineStoreDBStorePersistenceTypes for HybridOfflineStore support ([#6707](#6707)) ([310ab51](310ab51)), closes [#6701](#6701) * Add MLflow integration support to Feast operator ([#6611](#6611)) ([52999f1](52999f1)) * Add opt-in filter_by_created_timestamp cutoff to get_historical_features ([#6617](#6617)) ([79b33ce](79b33ce)), closes [#6615](#6615) * Add optional OIDC token audience and issuer verification ([#6670](#6670)) ([ef307c6](ef307c6)) * Add packaged feature repository support to Feast Operator ([8112b1e](8112b1e)), closes [#6598](#6598) * add plan() support to DynamoDBOnlineStore ([51ce982](51ce982)), closes [#6658](#6658) [#6659](#6659) * Added optional namespace/colleciton to datasets ([165fcf2](165fcf2)) * Added SQL registry schema_mode and registry create command ([#6704](#6704)) ([037c4cd](037c4cd)) * Allow users to have protected project on shared registry ([f9923bc](f9923bc)) * Apply Intermediate TLS defaults on API fallback and handle transient errors ([#6587](#6587)) ([43ae993](43ae993)) * **cli:** Updated feast init demo by adding rag template ([#5946](#5946)) ([c8628eb](c8628eb)), closes [#5264](#5264) * Expose the OIDC JWKS tunables through the operator ([#6690](#6690)) ([fef4e78](fef4e78)), closes [#6683](#6683) * Making feast vector store with open ai search api compatible ([#6121](#6121)) ([54da19a](54da19a)) * Multi-arch publish for feast operator image ([b221036](b221036)) * OpenLineage lineage enhancements - full object coverage, richer UI, and API-level sync ([#6719](#6719)) ([120a868](120a868)) * **operator:** Add spec.services.initImage for init container image override ([#6598](#6598)) ([ca355cb](ca355cb)) * Pass optional OIDC audience and issuer through the operator ([#6677](#6677)) ([a13ed7b](a13ed7b)), closes [#6670](#6670) * **server:** Remote Materialization ([#6649](#6649)) ([b7ae488](b7ae488)), closes [#4526](#4526) * Support Lineage configs via operator ([bf1e54a](bf1e54a)) * Updated datasets UI to support grouping ([7ae64ec](7ae64ec))

feast-dev#6709) * fix: feast apply ignores ttl updates when new ttl is None or timedelta(0) Fixes feast-dev#6703. Registry._update_metadata_fields() routes ttl changes on re-apply through a truthiness check: if (... and updated_fv.ttl): None and timedelta(0) are both the documented way to express "no ttl", and both are falsy, so re-applying a FeatureView/LabelView with ttl cleared silently kept the old finite ttl -- feast apply reported the update but nothing changed in the registry. Removing that outer truthy gate isn't sufficient by itself: for the FeatureView branch, get_ttl_duration() returns Python None when self.ttl is None, and the existing inner check if ttl_duration: existing_proto.spec.ttl.CopyFrom(ttl_duration) would still silently skip CopyFrom in that case, leaving the stale ttl in place. Fixed both by explicitly writing an empty Duration() (which decodes back to timedelta(0), per FeatureView.from_proto's existing ToNanoseconds()==0 check) whenever there's no real ttl to write, instead of skipping the write. The LabelView branch is adjusted the same way, guarding FromTimedelta() against a None ttl now that the outer gate no longer prevents ttl=None from reaching this branch. Traced all three cases (None, timedelta(0), a finite value) through the new branch logic in isolation and confirmed FeatureView and LabelView now resolve identically: None and timedelta(0) both produce a zero Duration, a finite ttl passes through unchanged. Signed-off-by: saket3395 <sakettulsan95@gmail.com> * address review nitpicks: hoist Duration import, guard FromTimedelta - Move the Duration import to the top of the file with the other google.protobuf imports, consistent with how Message and RepeatedCompositeFieldContainer are already imported there. - Wrap FromTimedelta() in the LabelView branch with a try/except, re-raising as a ValueError naming the offending value, so an invalid timedelta surfaces a clear error instead of a raw protobuf exception. Signed-off-by: saket3395 <sakettulsan95@gmail.com> * test: cover ttl clearing in _update_metadata_fields (feast-dev#6703) Adds unit coverage requested in review: - clearing a finite ttl to None or timedelta(0) now writes a zero Duration (previously silently dropped) - a finite-to-finite ttl update is preserved _update_metadata_fields uses no instance state, so it is exercised directly via the class, mirroring the FeatureView/FileSource/Field construction used elsewhere in the unit tests. Signed-off-by: saket3395 <sakettulsan95@gmail.com> * style: apply ruff format to registry.py ttl block Collapse the multi-line raise ValueError back to a single line per ruff format (it fits within the 88-char limit), fixing the format check. Signed-off-by: saket3395 <sakettulsan95@gmail.com> --------- Signed-off-by: saket3395 <sakettulsan95@gmail.com> Co-authored-by: saket3395 <sakettulsan95@gmail.com>

saket3395 requested a review from a team as a code owner August 6, 2026 06:37

jyejare approved these changes Aug 6, 2026

View reviewed changes

ntkathole added the ok-to-test label Aug 11, 2026

ntkathole changed the title Fix feast apply silently ignoring ttl updates to None or timedelta(0) Fix: Feast apply silently ignoring ttl updates to None or timedelta(0) Aug 14, 2026

ntkathole force-pushed the fix/ttl-clear-falsy-guard branch from 970b048 to 2c8a95d Compare August 14, 2026 03:21

saket3395 force-pushed the fix/ttl-clear-falsy-guard branch from 2c8a95d to 7dcb610 Compare August 14, 2026 03:39

saket3395 changed the title Fix: Feast apply silently ignoring ttl updates to None or timedelta(0) fix: Feast apply silently ignoring ttl updates to None or timedelta(0) Aug 14, 2026

saket3395 added 4 commits August 14, 2026 10:39

style: apply ruff format to registry.py ttl block

b34897a

Collapse the multi-line raise ValueError back to a single line per ruff format (it fits within the 88-char limit), fixing the format check. Signed-off-by: saket3395 <sakettulsan95@gmail.com>

ntkathole force-pushed the fix/ttl-clear-falsy-guard branch from 7dcb610 to b34897a Compare August 14, 2026 05:09

ntkathole approved these changes Aug 14, 2026

View reviewed changes

ntkathole merged commit 97b0f25 into feast-dev:master Aug 14, 2026
1 check passed

saket3395 mentioned this pull request Aug 14, 2026

feat: Add feature_service label to Feature Server RED metrics #6639

Open

+                      # are falsy, so that check would silently drop the update exactly
+                      # when a user clears an existing ttl.
+                      if hasattr(existing_proto.spec, "ttl") and hasattr(updated_fv, "ttl"):
+                          from google.protobuf.duration_pb2 import Duration

fix: Feast apply silently ignoring ttl updates to None or timedelta(0) by saket3395 · Pull Request #6709 · feast-dev/feast · GitHub

Uh oh!

fix: Feast apply silently ignoring ttl updates to None or timedelta(0) - #6709

fix: Feast apply silently ignoring ttl updates to None or timedelta(0)#6709
ntkathole merged 4 commits into
feast-dev:masterfrom
saket3395:fix/ttl-clear-falsy-guard

saket3395 commented Aug 6, 2026

Uh oh!

jyejare left a comment

Uh oh!

jyejare Aug 6, 2026

Uh oh!

jyejare Aug 6, 2026

Uh oh!

saket3395 commented Aug 6, 2026

Uh oh!

codecov-commenter commented Aug 11, 2026 •
edited
Loading

Uh oh!

Uh oh!

ntkathole commented Aug 11, 2026

Uh oh!

saket3395 commented Aug 11, 2026

Uh oh!

ntkathole commented Aug 14, 2026

Uh oh!

saket3395 commented Aug 14, 2026

Uh oh!

saket3395 commented Aug 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	from google.protobuf.duration_pb2 import Duration
	# Move this import to the top of the file with other imports
	from google.protobuf.duration_pb2 import Duration

fix: Feast apply silently ignoring ttl updates to None or timedelta(0) by saket3395 · Pull Request #6709 · feast-dev/feast · GitHub

Uh oh!

fix: Feast apply silently ignoring ttl updates to None or timedelta(0) - #6709

fix: Feast apply silently ignoring ttl updates to None or timedelta(0)#6709ntkathole merged 4 commits intofeast-dev:masterfeast-dev/feast:masterfrom saket3395:fix/ttl-clear-falsy-guardsaket3395/feast:fix/ttl-clear-falsy-guardCopy head branch name to clipboard

Conversation

saket3395 commented Aug 6, 2026

Summary

Fix

Verification

Test plan

Uh oh!

jyejare left a comment

Choose a reason for hiding this comment

Uh oh!

jyejare Aug 6, 2026

Choose a reason for hiding this comment

Uh oh!

jyejare Aug 6, 2026

Choose a reason for hiding this comment

Uh oh!

saket3395 commented Aug 6, 2026

Uh oh!

codecov-commenter commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ntkathole commented Aug 11, 2026

Uh oh!

saket3395 commented Aug 11, 2026

Uh oh!

ntkathole commented Aug 14, 2026

Uh oh!

saket3395 commented Aug 14, 2026

Uh oh!

saket3395 commented Aug 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

fix: Feast apply silently ignoring ttl updates to None or timedelta(0)#6709
ntkathole merged 4 commits into
feast-dev:masterfrom
saket3395:fix/ttl-clear-falsy-guard

codecov-commenter commented Aug 11, 2026 •
edited
Loading