Error raised when planning/applying a change that inserts/removes a push source

[hemidactylus]

Expected 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).

[hemidactylus]

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]

I'm getting this as well. Changing from a non push to a push source breaks on apply.

[adchia]

Making a PR for this. Thanks for catching!

[adchia]

@hemidactylus take a look! If you write /lgtm and also approve the PR, it'll merge. Thanks!

[hemidactylus]

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!