Skip to content

Commit 22e2c2a

Browse files
terryyylimterryyylim
committed
Allow entities and features to be updated
Signed-off-by: Terence <terencelimxp@gmail.com>
1 parent 83bb960 commit 22e2c2a

4 files changed

Lines changed: 48 additions & 67 deletions

File tree

‎core/src/main/java/feast/core/model/FeatureTable.java‎

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,6 @@
2525
import feast.proto.core.FeatureTableProto;
2626
import feast.proto.core.FeatureTableProto.FeatureTableSpec;
2727
import java.util.Collection;
28-
import java.util.Collections;
29-
import java.util.HashSet;
3028
import java.util.List;
3129
import java.util.Map;
3230
import java.util.Objects;
@@ -158,7 +156,8 @@ public static FeatureTable fromProto(
158156
* @param spec the Protobuf spec to update the FeatureTable from.
159157
* @throws IllegalArgumentException if the update will make prohibited changes.
160158
*/
161-
public void updateFromProto(FeatureTableSpec spec) {
159+
public void updateFromProto(
160+
String projectName, FeatureTableSpec spec, EntityRepository entityRepo) {
162161
// Check for prohibited changes made in spec:
163162
// - Name cannot be changed
164163
if (!getName().equals(spec.getName())) {
@@ -167,16 +166,11 @@ public void updateFromProto(FeatureTableSpec spec) {
167166
"Updating the name of a registered FeatureTable is not allowed: %s to %s",
168167
getName(), spec.getName()));
169168
}
170-
// - Entities cannot be changed
171-
List<String> entityNames =
172-
getEntities().stream().map(EntityV2::getName).collect(Collectors.toList());
173-
if (!new HashSet<>(entityNames).equals(new HashSet<>(spec.getEntitiesList()))) {
174-
Collections.sort(entityNames);
175-
throw new IllegalArgumentException(
176-
String.format(
177-
"Updating the entities of a registered FeatureTable is not allowed: %s to %s",
178-
entityNames, spec.getEntitiesList()));
179-
}
169+
// Update Entities if changed
170+
Set<EntityV2> entities =
171+
FeatureTable.resolveEntities(
172+
projectName, spec.getName(), entityRepo, spec.getEntitiesList());
173+
this.setEntities(entities);
180174

181175
// Update FeatureTable based on spec
182176
// Update existing features, create new feature, drop missing features

‎core/src/main/java/feast/core/model/FeatureV2.java‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,14 @@
2323
import java.util.Objects;
2424
import javax.persistence.*;
2525
import javax.persistence.Entity;
26+
import lombok.AccessLevel;
2627
import lombok.Getter;
28+
import lombok.Setter;
2729

2830
/** Defines a single Feature defined in a {@link FeatureTable} */
2931
@Getter
3032
@Entity
33+
@Setter(AccessLevel.PRIVATE)
3134
@Table(
3235
name = "features_v2",
3336
uniqueConstraints = @UniqueConstraint(columnNames = {"name", "feature_table_id"}))
@@ -96,12 +99,8 @@ public void updateFromProto(FeatureSpecV2 spec) {
9699
"Updating the name of a registered Feature is not allowed: %s to %s",
97100
getName(), spec.getName()));
98101
}
99-
if (!getType().equals(spec.getValueType())) {
100-
throw new IllegalArgumentException(
101-
String.format(
102-
"Updating the value type of a registered Feature is not allowed: %s to %s",
103-
getType(), spec.getValueType()));
104-
}
102+
// Update feature type
103+
this.setType(spec.getValueType());
105104

106105
// Update Feature based on spec
107106
this.labelsJSON = TypeConversion.convertMapToJsonString(spec.getLabelsMap());

‎core/src/main/java/feast/core/service/SpecService.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -671,7 +671,7 @@ public ApplyFeatureTableResponse applyFeatureTable(ApplyFeatureTableRequest requ
671671
return ApplyFeatureTableResponse.newBuilder().setTable(existingTable.get().toProto()).build();
672672
}
673673
if (existingTable.isPresent()) {
674-
existingTable.get().updateFromProto(applySpec);
674+
existingTable.get().updateFromProto(projectName, applySpec, entityRepository);
675675
table = existingTable.get();
676676
}
677677

‎core/src/test/java/feast/core/service/SpecServiceIT.java‎

Lines changed: 35 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1108,39 +1108,9 @@ public void shouldUpdateExistingTableWithValidSpec() {
11081108
}
11091109

11101110
@Test
1111-
public void shouldNotUpdateIfNoChanges() {
1112-
FeatureTableProto.FeatureTable table = apiClient.applyFeatureTable("default", getTestSpec());
1113-
FeatureTableProto.FeatureTable updatedTable =
1114-
apiClient.applyFeatureTable("default", getTestSpec());
1115-
1116-
assertThat(updatedTable.getMeta().getRevision(), equalTo(table.getMeta().getRevision()));
1117-
}
1118-
1119-
@Test
1120-
public void shouldErrorOnMissingBatchSource() {
1121-
FeatureTableProto.FeatureTableSpec spec =
1122-
DataGenerator.createFeatureTableSpec(
1123-
"ft",
1124-
List.of("entity1"),
1125-
Map.of("event_timestamp", ValueProto.ValueType.Enum.INT64),
1126-
3600,
1127-
Map.of())
1128-
.toBuilder()
1129-
.build();
1130-
1131-
StatusRuntimeException exc =
1132-
assertThrows(
1133-
StatusRuntimeException.class, () -> apiClient.applyFeatureTable("default", spec));
1134-
1135-
assertThat(
1136-
exc.getMessage(),
1137-
equalTo("INVALID_ARGUMENT: FeatureTable batch source cannot be empty."));
1138-
}
1139-
1140-
@Test
1141-
public void shouldErrorIfEntityChangeOnUpdate() {
1111+
public void shouldUpdateFeatureTableOnEntityChange() {
11421112
List<String> entities = Arrays.asList("entity1", "entity2");
1143-
FeatureTableProto.FeatureTableSpec spec =
1113+
FeatureTableProto.FeatureTableSpec updatedSpec =
11441114
DataGenerator.createFeatureTableSpec(
11451115
"featuretable1",
11461116
Arrays.asList("entity1"),
@@ -1157,21 +1127,15 @@ public void shouldErrorIfEntityChangeOnUpdate() {
11571127
DataGenerator.createFileDataSourceSpec("file:///path/to/file", "ts_col", ""))
11581128
.build();
11591129

1160-
StatusRuntimeException exc =
1161-
assertThrows(
1162-
StatusRuntimeException.class, () -> apiClient.applyFeatureTable("default", spec));
1130+
FeatureTableProto.FeatureTable updatedTable =
1131+
apiClient.applyFeatureTable("default", updatedSpec);
11631132

1164-
assertThat(
1165-
exc.getMessage(),
1166-
equalTo(
1167-
String.format(
1168-
"INVALID_ARGUMENT: Updating the entities of a registered FeatureTable is not allowed: %s to %s",
1169-
entities, spec.getEntitiesList())));
1133+
assertTrue(TestUtil.compareFeatureTableSpec(updatedTable.getSpec(), updatedSpec));
11701134
}
11711135

11721136
@Test
1173-
public void shouldErrorIfFeatureValueTypeChangeOnUpdate() {
1174-
FeatureTableProto.FeatureTableSpec spec =
1137+
public void shouldUpdateFeatureTableOnFeatureTypeChange() {
1138+
FeatureTableProto.FeatureTableSpec updatedSpec =
11751139
DataGenerator.createFeatureTableSpec(
11761140
"featuretable1",
11771141
Arrays.asList("entity1", "entity2"),
@@ -1188,16 +1152,40 @@ public void shouldErrorIfFeatureValueTypeChangeOnUpdate() {
11881152
DataGenerator.createFileDataSourceSpec("file:///path/to/file", "ts_col", ""))
11891153
.build();
11901154

1155+
FeatureTableProto.FeatureTable updatedTable =
1156+
apiClient.applyFeatureTable("default", updatedSpec);
1157+
1158+
assertTrue(TestUtil.compareFeatureTableSpec(updatedTable.getSpec(), updatedSpec));
1159+
}
1160+
1161+
@Test
1162+
public void shouldNotUpdateIfNoChanges() {
1163+
FeatureTableProto.FeatureTable table = apiClient.applyFeatureTable("default", getTestSpec());
1164+
FeatureTableProto.FeatureTable updatedTable =
1165+
apiClient.applyFeatureTable("default", getTestSpec());
1166+
1167+
assertThat(updatedTable.getMeta().getRevision(), equalTo(table.getMeta().getRevision()));
1168+
}
1169+
1170+
@Test
1171+
public void shouldErrorOnMissingBatchSource() {
1172+
FeatureTableProto.FeatureTableSpec spec =
1173+
DataGenerator.createFeatureTableSpec(
1174+
"ft",
1175+
List.of("entity1"),
1176+
Map.of("event_timestamp", ValueProto.ValueType.Enum.INT64),
1177+
3600,
1178+
Map.of())
1179+
.toBuilder()
1180+
.build();
1181+
11911182
StatusRuntimeException exc =
11921183
assertThrows(
11931184
StatusRuntimeException.class, () -> apiClient.applyFeatureTable("default", spec));
11941185

11951186
assertThat(
11961187
exc.getMessage(),
1197-
equalTo(
1198-
String.format(
1199-
"INVALID_ARGUMENT: Updating the value type of a registered Feature is not allowed: %s to %s",
1200-
ValueProto.ValueType.Enum.FLOAT, ValueProto.ValueType.Enum.STRING_LIST)));
1188+
equalTo("INVALID_ARGUMENT: FeatureTable batch source cannot be empty."));
12011189
}
12021190

12031191
@Test

0 commit comments

Comments
 (0)