GeezerVison is showing github.com — Error raised when planning/applying a change that inserts/removes a push source · Issue #3207 · feast-dev/feast
Error raised when planning/applying a change that inserts/removes a push source #3207DescriptionExpected Behavior
The command feast apply (or plan) is never supposed to raise an error. In particular, one may want to insert a push source into an existing repo, between a batch source and a Feature View: i.e. from (arrows here represent dependency in the feature repo definitions)
batch_source <-- feature_view
to
batch_source <-- push_source <-- feature_view
Current Behavior
An apply/plan invocation that leads Feast to compare a repo without the push source and one with the push source results in an error
being thrown within the PushSource class, specifically in its __eq__ method, because the "other" operand is None.
Steps to reproduce
Install Feast 0.24.1 and use the quickstart drivers example:
feast init repo_name
cd repo_name/feature_repo
feast apply
Now _alter the example_repo.py, changing the definition of driver_stats_fresh_fv so that its source parameter reads driver_stats_source instead of driver_stats_push_source: it is now
driver_stats_fresh_fv = FeatureView(
name="driver_hourly_stats_fresh",
entities=[driver],
ttl=timedelta(days=1),
schema=[
Field(name="conv_rate", dtype=Float32),
Field(name="acc_rate", dtype=Float32),
Field(name="avg_daily_trips", dtype=Int64),
],
online=True,
# THE FOLLOWING LINE EDITED instead of `driver_stats_push_source`:
source=driver_stats_source, # Changed from above
tags={"team": "driver_performance"},
)
Finally, try feast apply or feast plan, and the error is thrown. Stack:
File "/home/stefano/.virtualenvs/feast-pushsource-issue-39/bin/feast", line 8, in <module>
sys.exit(cli())
File "/home/stefano/.virtualenvs/feast-pushsource-issue-39/lib/python3.9/site-packages/click/core.py", line 1130, in __call__
return self.main(*args, **kwargs)
File "/home/stefano/.virtualenvs/feast-pushsource-issue-39/lib/python3.9/site-packages/click/core.py", line 1055, in main
rv = self.invoke(ctx)
File "/home/stefano/.virtualenvs/feast-pushsource-issue-39/lib/python3.9/site-packages/click/core.py", line 1657, in invoke
return _process_result(sub_ctx.command.invoke(sub_ctx))
File "/home/stefano/.virtualenvs/feast-pushsource-issue-39/lib/python3.9/site-packages/click/core.py", line 1404, in invoke
return ctx.invoke(self.callback, **ctx.params)
File "/home/stefano/.virtualenvs/feast-pushsource-issue-39/lib/python3.9/site-packages/click/core.py", line 760, in invoke
return __callback(*args, **kwargs)
File "/home/stefano/.virtualenvs/feast-pushsource-issue-39/lib/python3.9/site-packages/click/decorators.py", line 26, in new_func
return f(get_current_context(), *args, **kwargs)
File "/home/stefano/.virtualenvs/feast-pushsource-issue-39/lib/python3.9/site-packages/feast/cli.py", line 519, in apply_total_command
apply_total(repo_config, repo, skip_source_validation)
File "/home/stefano/.virtualenvs/feast-pushsource-issue-39/lib/python3.9/site-packages/feast/usage.py", line 283, in wrapper
return func(*args, **kwargs)
File "/home/stefano/.virtualenvs/feast-pushsource-issue-39/lib/python3.9/site-packages/feast/repo_operations.py", line 335, in apply_total
apply_total_with_repo_instance(
File "/home/stefano/.virtualenvs/feast-pushsource-issue-39/lib/python3.9/site-packages/feast/repo_operations.py", line 296, in apply_total_with_repo_instance
registry_diff, infra_diff, new_infra = store.plan(repo)
File "/home/stefano/.virtualenvs/feast-pushsource-issue-39/lib/python3.9/site-packages/feast/usage.py", line 294, in wrapper
raise exc.with_traceback(traceback)
File "/home/stefano/.virtualenvs/feast-pushsource-issue-39/lib/python3.9/site-packages/feast/usage.py", line 283, in wrapper
return func(*args, **kwargs)
File "/home/stefano/.virtualenvs/feast-pushsource-issue-39/lib/python3.9/site-packages/feast/feature_store.py", line 734, in plan
registry_diff = diff_between(
File "/home/stefano/.virtualenvs/feast-pushsource-issue-39/lib/python3.9/site-packages/feast/diff/registry_diff.py", line 289, in diff_between
diff_registry_objects(current_obj, e, object_type)
File "/home/stefano/.virtualenvs/feast-pushsource-issue-39/lib/python3.9/site-packages/feast/diff/registry_diff.py", line 142, in diff_registry_objects
if current != new:
File "/home/stefano/.virtualenvs/feast-pushsource-issue-39/lib/python3.9/site-packages/feast/feature_view.py", line 249, in __eq__
or self.stream_source != other.stream_source
File "/home/stefano/.virtualenvs/feast-pushsource-issue-39/lib/python3.9/site-packages/feast/data_source.py", line 763, in __eq__
raise TypeError("Comparisons should only involve PushSource class objects.")
TypeError: Comparisons should only involve PushSource class objects.
Specifications
- Version: Feast v0.24.1
- Platform: Linux (ubuntu 20) with Python 3.9
- Subsystem:
Possible Solution
When comparing "existing" and "desired" repo structure, the push sources are
compared (old vs new) to check if there are changes.
But for PushSource (as well as for KafkaSource, RequestSource and KinesisSource for that matter)
the comparison breaks with the error above in their __eq__ method: the first thing
happening in the method is "raise error, unless isinstance(other, PushSource)".
This is at odds with the superclass DataSource, whose __eq__ starts with: if other is None: return False. If subclasses had this,
no errors would be raised and the feature repo's structure could have been modified (in particular, inserting/removing
a push source between a batch source and a feature view, at will). Activityhemidactylus commented on Sep 13, 2022 ContributorAuthor More actions In the above example, the error occurs when removing a push source, while in "expected behaviour" I refer to adding one: to clarify, I want to add that the same error is thrown in both cases. robhowley commented on Sep 14, 2022 I'm getting this as well. Changing from a non push to a push source breaks on apply. adchia commented on Sep 14, 2022 Making a PR for this. Thanks for catching! adchia commented on Sep 14, 2022 @hemidactylus take a look! If you write /lgtm and also approve the PR, it'll merge. Thanks! hemidactylus commented on Sep 22, 2022 ContributorAuthor More actions Thank you! But it seems I cannot /lgtm the PR (which is approved already). Sorry for coming late to the party, I'll be able to devote more time to this again moving forward!
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|