Skip to content

Commit 0f149a9

Browse files
larrysingleton007larrysingleton007
authored
fix: Return False from __eq__ on cross-type comparison (#6637)
* fix: Return False from __eq__ on cross-type comparison Every registry object's __eq__ did `if not isinstance(other, X): raise TypeError("Comparisons should only involve X class objects.")`. Raising from __eq__ on an operand of a different type is wrong: comparing objects of different types should be False, not an error. As a result, changing a feature view's data source type and re-applying over an existing registry crashed with `TypeError: Comparisons should only involve <X> class objects.` Replace the raise with `return False` across all 35 affected __eq__ methods, matching PushSource.__eq__ which already handles a cross-type comparison this way. Update the LabelView equality test (it asserted the removed TypeError) and add cross-type regression tests for DataSource, Entity, and policy objects, all asserting False rather than a raise. Signed-off-by: Larry Singleton <[email protected]> * test: Add parametrized cross-type __eq__ coverage across the object model Assert that cross-type comparison returns False (never raises) for the importable core types touched by the sweep: Entity, Project, Aggregation, Permission, all four policies, the Kafka/Kinesis/Request and File/BigQuery/Redshift/Snowflake sources, FeatureView, and FeatureService. Complements the existing per-type tests (DataSource, Entity, LabelView, RoleBasedPolicy) and raises patch coverage on the sweep. Contrib sources and optional-dependency transformations are omitted since the unit env can't import them. Signed-off-by: Larry Singleton <[email protected]> * fix: Guard SparkSource, Feature, and data format __eq__ against cross-type comparison SparkSource.__eq__ passed the shared DataSource base check for any DataSource subclass and then accessed spark-only attributes, so comparing against a FileSource with a matching name raised AttributeError instead of returning False (reported on #6636 when swapping a FeatureView's source from FileSource to SparkSource). Feature.__eq__ and the FileFormat/StreamFormat __eq__ accessed attributes of the other operand unguarded and failed the same way; they raised AttributeError rather than the TypeError pattern, which is why the original sweep missed them. Signed-off-by: Larry Singleton <[email protected]> --------- Signed-off-by: Larry Singleton <[email protected]>
1 parent abffebc commit 0f149a9

37 files changed

Lines changed: 214 additions & 91 deletions

‎sdk/python/feast/aggregation/__init__.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ def from_proto(cls, agg_proto: AggregationProto):
9090

9191
def __eq__(self, other):
9292
if not isinstance(other, Aggregation):
93-
raise TypeError("Comparisons should only involve Aggregations.")
93+
return False
9494

9595
if (
9696
self.column != other.column

‎sdk/python/feast/base_feature_view.py‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -172,9 +172,7 @@ def _schema_or_udf_changed(self, other: "BaseFeatureView") -> bool:
172172

173173
def __eq__(self, other):
174174
if not isinstance(other, BaseFeatureView):
175-
raise TypeError(
176-
"Comparisons should only involve BaseFeatureView class objects."
177-
)
175+
return False
178176

179177
if (
180178
self.name != other.name

‎sdk/python/feast/data_format.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ def to_proto(self):
3333
pass
3434

3535
def __eq__(self, other):
36+
if not isinstance(other, FileFormat):
37+
return False
3638
return self.to_proto() == other.to_proto()
3739

3840
@classmethod
@@ -95,6 +97,8 @@ def to_proto(self):
9597
pass
9698

9799
def __eq__(self, other):
100+
if not isinstance(other, StreamFormat):
101+
return False
98102
return self.to_proto() == other.to_proto()
99103

100104
@classmethod

‎sdk/python/feast/data_source.py‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ def __eq__(self, other):
279279
return False
280280

281281
if not isinstance(other, DataSource):
282-
raise TypeError("Comparisons should only involve DataSource class objects.")
282+
return False
283283

284284
if (
285285
self.name != other.name
@@ -499,9 +499,7 @@ def __init__(
499499

500500
def __eq__(self, other):
501501
if not isinstance(other, KafkaSource):
502-
raise TypeError(
503-
"Comparisons should only involve KafkaSource class objects."
504-
)
502+
return False
505503

506504
if not super().__eq__(other):
507505
return False
@@ -639,9 +637,7 @@ def get_table_column_names_and_types(
639637

640638
def __eq__(self, other):
641639
if not isinstance(other, RequestSource):
642-
raise TypeError(
643-
"Comparisons should only involve RequestSource class objects."
644-
)
640+
return False
645641

646642
if not super().__eq__(other):
647643
return False
@@ -801,9 +797,7 @@ def __init__(
801797

802798
def __eq__(self, other):
803799
if not isinstance(other, KinesisSource):
804-
raise TypeError(
805-
"Comparisons should only involve KinesisSource class objects."
806-
)
800+
return False
807801

808802
if not super().__eq__(other):
809803
return False

‎sdk/python/feast/entity.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ def __hash__(self) -> int:
126126

127127
def __eq__(self, other):
128128
if not isinstance(other, Entity):
129-
raise TypeError("Comparisons should only involve Entity class objects.")
129+
return False
130130

131131
if (
132132
self.name != other.name

‎sdk/python/feast/feature.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ def __init__(
5050
self._labels = labels
5151

5252
def __eq__(self, other):
53+
if not isinstance(other, Feature):
54+
return False
5355
if self.name != other.name or self.dtype != other.dtype:
5456
return False
5557
return True

‎sdk/python/feast/feature_service.py‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -293,9 +293,7 @@ def __hash__(self):
293293

294294
def __eq__(self, other):
295295
if not isinstance(other, FeatureService):
296-
raise TypeError(
297-
"Comparisons should only involve FeatureService class objects."
298-
)
296+
return False
299297

300298
if (
301299
self.name != other.name

‎sdk/python/feast/feature_view.py‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -395,9 +395,7 @@ def _schema_or_udf_changed(self, other: "BaseFeatureView") -> bool:
395395

396396
def __eq__(self, other):
397397
if not isinstance(other, FeatureView):
398-
raise TypeError(
399-
"Comparisons should only involve FeatureView class objects."
400-
)
398+
return False
401399

402400
if not super().__eq__(other):
403401
return False

‎sdk/python/feast/infra/offline_stores/bigquery_source.py‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,9 +97,7 @@ def __hash__(self):
9797

9898
def __eq__(self, other):
9999
if not isinstance(other, BigQuerySource):
100-
raise TypeError(
101-
"Comparisons should only involve BigQuerySource class objects."
102-
)
100+
return False
103101

104102
return (
105103
super().__eq__(other)

‎sdk/python/feast/infra/offline_stores/contrib/athena_offline_store/athena_source.py‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,7 @@ def __hash__(self):
115115

116116
def __eq__(self, other):
117117
if not isinstance(other, AthenaSource):
118-
raise TypeError(
119-
"Comparisons should only involve AthenaSource class objects."
120-
)
118+
return False
121119

122120
return (
123121
super().__eq__(other)

0 commit comments

Comments
 (0)