Skip to content

Commit 77a83fd

Browse files
terryyylimterryyylim
authored
Update Feast Core list features method (feast-dev#1176)
* Update listFeatures method in core Signed-off-by: Terence <[email protected]> * Update tests Signed-off-by: Terence <[email protected]> * Address PR comments Signed-off-by: Terence <[email protected]> * Update python SDK Signed-off-by: Terence <[email protected]> * Make integration test clearer Signed-off-by: Terence <[email protected]>
1 parent 912376c commit 77a83fd

11 files changed

Lines changed: 274 additions & 77 deletions

File tree

‎common-test/src/main/java/feast/common/it/SimpleCoreClient.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ public void updateFeatureSetStatus(
162162
.build());
163163
}
164164

165-
public Map<String, FeatureSetProto.FeatureSpec> simpleListFeatures(
165+
public Map<String, FeatureProto.FeatureSpecV2> simpleListFeatures(
166166
String projectName, Map<String, String> labels, List<String> entities) {
167167
return stub.listFeatures(
168168
CoreServiceProto.ListFeaturesRequest.newBuilder()
@@ -176,7 +176,7 @@ public Map<String, FeatureSetProto.FeatureSpec> simpleListFeatures(
176176
.getFeaturesMap();
177177
}
178178

179-
public Map<String, FeatureSetProto.FeatureSpec> simpleListFeatures(
179+
public Map<String, FeatureProto.FeatureSpecV2> simpleListFeatures(
180180
String projectName, String... entities) {
181181
return simpleListFeatures(projectName, Collections.emptyMap(), Arrays.asList(entities));
182182
}

‎core/src/main/java/feast/core/controller/CoreServiceRestController.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ public ListFeatureSetsResponse listFeatureSets(
114114
* <code>default</code>.
115115
* @return (200 OK) Return {@link ListFeaturesResponse} in JSON.
116116
*/
117-
@RequestMapping(value = "/v1/features", method = RequestMethod.GET)
117+
@RequestMapping(value = "/v2/features", method = RequestMethod.GET)
118118
public ListFeaturesResponse listFeatures(
119119
@RequestParam String[] entities, @RequestParam(required = false) Optional<String> project) {
120120
ListFeaturesRequest.Filter.Builder filterBuilder =

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

Lines changed: 70 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
*/
1717
package feast.core.model;
1818

19+
import static feast.common.models.FeatureV2.getFeatureStringRef;
20+
1921
import com.google.common.hash.Hashing;
2022
import com.google.protobuf.Duration;
2123
import com.google.protobuf.Timestamp;
@@ -25,6 +27,7 @@
2527
import feast.proto.core.FeatureProto.FeatureSpecV2;
2628
import feast.proto.core.FeatureTableProto;
2729
import feast.proto.core.FeatureTableProto.FeatureTableSpec;
30+
import feast.proto.serving.ServingAPIProto;
2831
import java.util.*;
2932
import java.util.stream.Collectors;
3033
import javax.persistence.CascadeType;
@@ -73,7 +76,7 @@ public class FeatureTable extends AbstractTimestampEntity {
7376
private Set<FeatureV2> features;
7477

7578
// Entites to associate the features defined in this FeatureTable with
76-
@ManyToMany
79+
@ManyToMany(fetch = FetchType.EAGER)
7780
@JoinTable(
7881
name = "feature_tables_entities_v2",
7982
joinColumns = @JoinColumn(name = "feature_table_id"),
@@ -263,6 +266,72 @@ private static Set<EntityV2> resolveEntities(
263266
.collect(Collectors.toSet());
264267
}
265268

269+
/**
270+
* Return a boolean to indicate if FeatureTable contains all specified entities.
271+
*
272+
* @param entitiesFilter contain entities that should be attached to the FeatureTable
273+
* @return boolean True if FeatureTable contains all entities in the entitiesFilter
274+
*/
275+
public boolean hasAllEntities(List<String> entitiesFilter) {
276+
Set<String> allEntitiesName =
277+
this.getEntities().stream().map(entity -> entity.getName()).collect(Collectors.toSet());
278+
return allEntitiesName.equals(new HashSet<>(entitiesFilter));
279+
}
280+
281+
/**
282+
* Returns a map of Feature references and Features if FeatureTable's Feature contains all labels
283+
* in the labelsFilter
284+
*
285+
* @param labelsFilter contain labels that should be attached to FeatureTable's features
286+
* @return Map of Feature references and Features
287+
*/
288+
public Map<String, FeatureV2> getFeaturesByLabels(Map<String, String> labelsFilter) {
289+
Map<String, FeatureV2> validFeaturesMap;
290+
List<FeatureV2> validFeatures;
291+
if (labelsFilter.size() > 0) {
292+
validFeatures = filterFeaturesByAllLabels(this.getFeatures(), labelsFilter);
293+
validFeaturesMap = getFeaturesRefToFeaturesMap(validFeatures);
294+
return validFeaturesMap;
295+
}
296+
validFeaturesMap = getFeaturesRefToFeaturesMap(List.copyOf(this.getFeatures()));
297+
return validFeaturesMap;
298+
}
299+
300+
/**
301+
* Returns map for accessing features using their respective feature reference.
302+
*
303+
* @param features List of features to insert to map.
304+
* @return Map of featureRef:feature.
305+
*/
306+
private Map<String, FeatureV2> getFeaturesRefToFeaturesMap(List<FeatureV2> features) {
307+
Map<String, FeatureV2> validFeaturesMap = new HashMap<>();
308+
for (FeatureV2 feature : features) {
309+
ServingAPIProto.FeatureReferenceV2 featureRef =
310+
ServingAPIProto.FeatureReferenceV2.newBuilder()
311+
.setFeatureTable(this.getName())
312+
.setName(feature.getName())
313+
.build();
314+
validFeaturesMap.put(getFeatureStringRef(featureRef), feature);
315+
}
316+
return validFeaturesMap;
317+
}
318+
319+
/**
320+
* Returns a list of Features if FeatureTable's Feature contains all labels in labelsFilter
321+
*
322+
* @param labelsFilter contain labels that should be attached to FeatureTable's features
323+
* @return List of Features
324+
*/
325+
public static List<FeatureV2> filterFeaturesByAllLabels(
326+
Set<FeatureV2> features, Map<String, String> labelsFilter) {
327+
List<FeatureV2> validFeatures =
328+
features.stream()
329+
.filter(feature -> feature.hasAllLabels(labelsFilter))
330+
.collect(Collectors.toList());
331+
332+
return validFeatures;
333+
}
334+
266335
/**
267336
* Determine whether a FeatureTable has all the specified labels.
268337
*

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,23 @@ public void updateFromProto(FeatureSpecV2 spec) {
106106
this.labelsJSON = TypeConversion.convertMapToJsonString(spec.getLabelsMap());
107107
}
108108

109+
/**
110+
* Return a boolean to indicate if Feature contains all specified labels.
111+
*
112+
* @param labelsFilter contain labels that should be attached to Feature
113+
* @return boolean True if Feature contains all labels in the labelsFilter
114+
*/
115+
public boolean hasAllLabels(Map<String, String> labelsFilter) {
116+
Map<String, String> featureLabelsMap = TypeConversion.convertJsonStringToMap(getLabelsJSON());
117+
for (String key : labelsFilter.keySet()) {
118+
if (!featureLabelsMap.containsKey(key)
119+
|| !featureLabelsMap.get(key).equals(labelsFilter.get(key))) {
120+
return false;
121+
}
122+
}
123+
return true;
124+
}
125+
109126
@Override
110127
public int hashCode() {
111128
return Objects.hash(getName(), getType(), getLabelsJSON());

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

Lines changed: 24 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -297,46 +297,37 @@ public ListFeatureSetsResponse listFeatureSets(ListFeatureSetsRequest.Filter fil
297297
* filter
298298
*/
299299
public ListFeaturesResponse listFeatures(ListFeaturesRequest.Filter filter) {
300-
try {
301-
String project = filter.getProject();
302-
List<String> entities = filter.getEntitiesList();
303-
Map<String, String> labels = filter.getLabelsMap();
300+
String project = filter.getProject();
301+
List<String> entities = filter.getEntitiesList();
302+
Map<String, String> labels = filter.getLabelsMap();
304303

305-
checkValidCharactersAllowAsterisk(project, "project");
304+
checkValidCharactersAllowAsterisk(project, "project");
306305

307-
// Autofill default project if project not specified
308-
if (project.isEmpty()) {
309-
project = Project.DEFAULT_NAME;
310-
}
306+
// Autofill default project if project not specified
307+
if (project.isEmpty()) {
308+
project = Project.DEFAULT_NAME;
309+
}
311310

312-
// Currently defaults to all FeatureSets
313-
List<FeatureSet> featureSets =
314-
featureSetRepository.findAllByNameLikeAndProject_NameOrderByNameAsc("%", project);
315-
// TODO: List features in Feature Tables.
311+
// Currently defaults to all FeatureTables
312+
List<FeatureTable> featureTables = tableRepository.findAllByProject_Name(project);
316313

317-
ListFeaturesResponse.Builder response = ListFeaturesResponse.newBuilder();
318-
if (entities.size() > 0) {
319-
featureSets =
320-
featureSets.stream()
321-
.filter(featureSet -> featureSet.hasAllEntities(entities))
322-
.collect(Collectors.toList());
323-
}
314+
ListFeaturesResponse.Builder response = ListFeaturesResponse.newBuilder();
315+
if (entities.size() > 0) {
316+
featureTables =
317+
featureTables.stream()
318+
.filter(featureTable -> featureTable.hasAllEntities(entities))
319+
.collect(Collectors.toList());
320+
}
324321

325-
Map<String, Feature> featuresMap;
326-
for (FeatureSet featureSet : featureSets) {
327-
featuresMap = featureSet.getFeaturesByRef(labels);
328-
for (Map.Entry<String, Feature> entry : featuresMap.entrySet()) {
329-
response.putFeatures(entry.getKey(), entry.getValue().toProto());
330-
}
322+
Map<String, FeatureV2> featuresMap;
323+
for (FeatureTable featureTable : featureTables) {
324+
featuresMap = featureTable.getFeaturesByLabels(labels);
325+
for (Map.Entry<String, FeatureV2> entry : featuresMap.entrySet()) {
326+
response.putFeatures(entry.getKey(), entry.getValue().toProto());
331327
}
332-
333-
return response.build();
334-
} catch (InvalidProtocolBufferException e) {
335-
throw io.grpc.Status.NOT_FOUND
336-
.withDescription("Unable to retrieve features")
337-
.withCause(e)
338-
.asRuntimeException();
339328
}
329+
330+
return response.build();
340331
}
341332

342333
/**

‎core/src/test/java/feast/core/controller/CoreServiceRestIT.java‎

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -176,12 +176,9 @@ public void listFeatureSets() {
176176

177177
@Test
178178
public void listFeatures() {
179-
// entities = [merchant_id]
180-
// project = default
181-
// should return 4 features
182179
String uri1 =
183-
UriComponentsBuilder.fromPath("/api/v1/features")
184-
.queryParam("entities", "merchant_id")
180+
UriComponentsBuilder.fromPath("/api/v2/features")
181+
.queryParam("entities", "entity1", "entity2")
185182
.buildAndExpand()
186183
.toString();
187184
get(uri1)
@@ -190,15 +187,12 @@ public void listFeatures() {
190187
.everything()
191188
.assertThat()
192189
.contentType(ContentType.JSON)
193-
.body("features", aMapWithSize(4));
190+
.body("features", aMapWithSize(2));
194191

195-
// entities = [merchant_id]
196-
// project = merchant
197-
// should return 2 features
198192
String uri2 =
199-
UriComponentsBuilder.fromPath("/api/v1/features")
200-
.queryParam("entities", "merchant_id")
201-
.queryParam("project", "merchant")
193+
UriComponentsBuilder.fromPath("/api/v2/features")
194+
.queryParam("entities", "entity1", "entity2")
195+
.queryParam("project", "default")
202196
.buildAndExpand()
203197
.toString();
204198
get(uri2)

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

Lines changed: 53 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,23 @@ public void initState() {
102102
.setBatchSource(
103103
DataGenerator.createFileDataSourceSpec("file:///path/to/file", "ts_col", ""))
104104
.build());
105+
apiClient.applyFeatureTable(
106+
"default",
107+
DataGenerator.createFeatureTableSpec(
108+
"featuretable2",
109+
Arrays.asList("entity1", "entity2"),
110+
new HashMap<>() {
111+
{
112+
put("feature3", ValueProto.ValueType.Enum.STRING);
113+
put("feature4", ValueProto.ValueType.Enum.FLOAT);
114+
}
115+
},
116+
7200,
117+
ImmutableMap.of("feat_key4", "feat_value4"))
118+
.toBuilder()
119+
.setBatchSource(
120+
DataGenerator.createFileDataSourceSpec("file:///path/to/file", "ts_col", ""))
121+
.build());
105122
apiClient.simpleApplyEntity(
106123
"project1",
107124
DataGenerator.createEntitySpecV2(
@@ -312,10 +329,13 @@ public void shouldUseDefaultProjectIfProjectUnspecified() {
312329
List<FeatureTableProto.FeatureTable> featureTables =
313330
apiClient.simpleListFeatureTables(filter);
314331

315-
assertThat(featureTables, hasSize(1));
332+
assertThat(featureTables, hasSize(2));
316333
assertThat(
317334
featureTables,
318335
hasItem(hasProperty("spec", hasProperty("name", equalTo("featuretable1")))));
336+
assertThat(
337+
featureTables,
338+
hasItem(hasProperty("spec", hasProperty("name", equalTo("featuretable2")))));
319339
}
320340

321341
@Test
@@ -1005,49 +1025,55 @@ class ListFeatures {
10051025
@Test
10061026
public void shouldFilterFeaturesByEntitiesAndLabels() {
10071027
// Case 1: Only filter by entities
1008-
Map<String, FeatureSetProto.FeatureSpec> result1 =
1009-
apiClient.simpleListFeatures("project1", "user_id");
1028+
Map<String, FeatureProto.FeatureSpecV2> result1 =
1029+
apiClient.simpleListFeatures("default", "entity1", "entity2");
10101030

1011-
assertThat(result1, aMapWithSize(2));
1012-
assertThat(result1, hasKey(equalTo("project1/fs3:feature1")));
1013-
assertThat(result1, hasKey(equalTo("project1/fs3:feature2")));
1031+
assertThat(result1, aMapWithSize(4));
1032+
assertThat(result1, hasKey(equalTo("featuretable1:feature1")));
1033+
assertThat(result1, hasKey(equalTo("featuretable1:feature2")));
1034+
assertThat(result1, hasKey(equalTo("featuretable2:feature3")));
1035+
assertThat(result1, hasKey(equalTo("featuretable2:feature4")));
10141036

10151037
// Case 2: Filter by entities and labels
1016-
Map<String, FeatureSetProto.FeatureSpec> result2 =
1038+
Map<String, FeatureProto.FeatureSpecV2> result2 =
10171039
apiClient.simpleListFeatures(
1018-
"project1",
1019-
ImmutableMap.of("app", "feast", "version", "one"),
1020-
ImmutableList.of("customer_id"));
1040+
"default",
1041+
ImmutableMap.of("feat_key2", "feat_value2"),
1042+
ImmutableList.of("entity1", "entity2"));
10211043

1022-
assertThat(result2, aMapWithSize(1));
1023-
assertThat(result2, hasKey(equalTo("project1/fs4:feature2")));
1044+
assertThat(result2, aMapWithSize(2));
1045+
assertThat(result2, hasKey(equalTo("featuretable1:feature1")));
1046+
assertThat(result2, hasKey(equalTo("featuretable1:feature2")));
10241047

10251048
// Case 3: Filter by labels
1026-
Map<String, FeatureSetProto.FeatureSpec> result3 =
1049+
Map<String, FeatureProto.FeatureSpecV2> result3 =
10271050
apiClient.simpleListFeatures(
1028-
"project1", ImmutableMap.of("app", "feast"), Collections.emptyList());
1051+
"default", ImmutableMap.of("feat_key4", "feat_value4"), Collections.emptyList());
10291052

10301053
assertThat(result3, aMapWithSize(2));
1031-
assertThat(result3, hasKey(equalTo("project1/fs4:feature2")));
1032-
assertThat(result3, hasKey(equalTo("project1/fs5:feature3")));
1054+
assertThat(result3, hasKey(equalTo("featuretable2:feature3")));
1055+
assertThat(result3, hasKey(equalTo("featuretable2:feature4")));
10331056

10341057
// Case 4: Filter by nothing, except project
1035-
Map<String, FeatureSetProto.FeatureSpec> result4 =
1058+
Map<String, FeatureProto.FeatureSpecV2> result4 =
10361059
apiClient.simpleListFeatures("project1", ImmutableMap.of(), Collections.emptyList());
10371060

1038-
assertThat(result4, aMapWithSize(4));
1039-
assertThat(result4, hasKey(equalTo("project1/fs3:feature1")));
1040-
assertThat(result4, hasKey(equalTo("project1/fs3:feature1")));
1041-
assertThat(result4, hasKey(equalTo("project1/fs4:feature2")));
1042-
assertThat(result4, hasKey(equalTo("project1/fs5:feature3")));
1061+
assertThat(result4, aMapWithSize(0));
10431062

10441063
// Case 5: Filter by nothing; will use default project
1045-
Map<String, FeatureSetProto.FeatureSpec> result5 =
1064+
Map<String, FeatureProto.FeatureSpecV2> result5 =
10461065
apiClient.simpleListFeatures("", ImmutableMap.of(), Collections.emptyList());
10471066

1048-
assertThat(result5, aMapWithSize(2));
1049-
assertThat(result5, hasKey(equalTo("default/fs1:total")));
1050-
assertThat(result5, hasKey(equalTo("default/fs2:sum")));
1067+
assertThat(result5, aMapWithSize(4));
1068+
assertThat(result5, hasKey(equalTo("featuretable1:feature1")));
1069+
assertThat(result5, hasKey(equalTo("featuretable1:feature2")));
1070+
assertThat(result5, hasKey(equalTo("featuretable2:feature3")));
1071+
assertThat(result5, hasKey(equalTo("featuretable2:feature4")));
1072+
1073+
// Case 6: Filter by mismatched entity
1074+
Map<String, FeatureProto.FeatureSpecV2> result6 =
1075+
apiClient.simpleListFeatures("default", ImmutableMap.of(), ImmutableList.of("entity1"));
1076+
assertThat(result6, aMapWithSize(0));
10511077
}
10521078
}
10531079

@@ -1350,6 +1376,7 @@ public void shouldReturnNoTables() {
13501376
CoreServiceProto.ListFeatureTablesRequest.Filter filter =
13511377
CoreServiceProto.ListFeatureTablesRequest.Filter.newBuilder()
13521378
.setProject("default")
1379+
.putLabels("feat_key2", "feat_value2")
13531380
.build();
13541381
List<FeatureTableProto.FeatureTable> featureTables =
13551382
apiClient.simpleListFeatureTables(filter);

0 commit comments

Comments
 (0)