Skip to content

Commit

Permalink
Update Feast Core list features method (#1176)
Browse files Browse the repository at this point in the history 
* Update listFeatures method in core

Signed-off-by: Terence <terencelimxp@gmail.com>

* Update tests

Signed-off-by: Terence <terencelimxp@gmail.com>

* Address PR comments

Signed-off-by: Terence <terencelimxp@gmail.com>

* Update python SDK

Signed-off-by: Terence <terencelimxp@gmail.com>

* Make integration test clearer

Signed-off-by: Terence <terencelimxp@gmail.com>
  • Loading branch information
@terryyylim
terryyylim authored Nov 20, 2020
1 parent 912376c commit 77a83fd
Show file tree
Hide file tree
Showing 11 changed files with 274 additions and 77 deletions.
4 changes: 2 additions & 2 deletions common-test/src/main/java/feast/common/it/SimpleCoreClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ public void updateFeatureSetStatus(
.build());
}

public Map<String, FeatureSetProto.FeatureSpec> simpleListFeatures(
public Map<String, FeatureProto.FeatureSpecV2> simpleListFeatures(
String projectName, Map<String, String> labels, List<String> entities) {
return stub.listFeatures(
CoreServiceProto.ListFeaturesRequest.newBuilder()
Expand All @@ -176,7 +176,7 @@ public Map<String, FeatureSetProto.FeatureSpec> simpleListFeatures(
.getFeaturesMap();
}

public Map<String, FeatureSetProto.FeatureSpec> simpleListFeatures(
public Map<String, FeatureProto.FeatureSpecV2> simpleListFeatures(
String projectName, String... entities) {
return simpleListFeatures(projectName, Collections.emptyMap(), Arrays.asList(entities));
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/feast/core/controller/CoreServiceRestController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ public ListFeatureSetsResponse listFeatureSets(
* <code>default</code>.
* @return (200 OK) Return {@link ListFeaturesResponse} in JSON.
*/
@RequestMapping(value = "/v1/features", method = RequestMethod.GET)
@RequestMapping(value = "/v2/features", method = RequestMethod.GET)
public ListFeaturesResponse listFeatures(
@RequestParam String[] entities, @RequestParam(required = false) Optional<String> project) {
ListFeaturesRequest.Filter.Builder filterBuilder =
Expand Down
71 changes: 70 additions & 1 deletion core/src/main/java/feast/core/model/FeatureTable.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
*/
package feast.core.model;

import static feast.common.models.FeatureV2.getFeatureStringRef;

import com.google.common.hash.Hashing;
import com.google.protobuf.Duration;
import com.google.protobuf.Timestamp;
Expand All @@ -25,6 +27,7 @@
import feast.proto.core.FeatureProto.FeatureSpecV2;
import feast.proto.core.FeatureTableProto;
import feast.proto.core.FeatureTableProto.FeatureTableSpec;
import feast.proto.serving.ServingAPIProto;
import java.util.*;
import java.util.stream.Collectors;
import javax.persistence.CascadeType;
Expand Down Expand Up @@ -73,7 +76,7 @@ public class FeatureTable extends AbstractTimestampEntity {
private Set<FeatureV2> features;

// Entites to associate the features defined in this FeatureTable with
@ManyToMany
@ManyToMany(fetch = FetchType.EAGER)
@JoinTable(
name = "feature_tables_entities_v2",
joinColumns = @JoinColumn(name = "feature_table_id"),
Expand Down Expand Up @@ -263,6 +266,72 @@ private static Set<EntityV2> resolveEntities(
.collect(Collectors.toSet());
}

/**
* Return a boolean to indicate if FeatureTable contains all specified entities.
*
* @param entitiesFilter contain entities that should be attached to the FeatureTable
* @return boolean True if FeatureTable contains all entities in the entitiesFilter
*/
public boolean hasAllEntities(List<String> entitiesFilter) {
Set<String> allEntitiesName =
this.getEntities().stream().map(entity -> entity.getName()).collect(Collectors.toSet());
return allEntitiesName.equals(new HashSet<>(entitiesFilter));
}

/**
* Returns a map of Feature references and Features if FeatureTable's Feature contains all labels
* in the labelsFilter
*
* @param labelsFilter contain labels that should be attached to FeatureTable's features
* @return Map of Feature references and Features
*/
public Map<String, FeatureV2> getFeaturesByLabels(Map<String, String> labelsFilter) {
Map<String, FeatureV2> validFeaturesMap;
List<FeatureV2> validFeatures;
if (labelsFilter.size() > 0) {
validFeatures = filterFeaturesByAllLabels(this.getFeatures(), labelsFilter);
validFeaturesMap = getFeaturesRefToFeaturesMap(validFeatures);
return validFeaturesMap;
}
validFeaturesMap = getFeaturesRefToFeaturesMap(List.copyOf(this.getFeatures()));
return validFeaturesMap;
}

/**
* Returns map for accessing features using their respective feature reference.
*
* @param features List of features to insert to map.
* @return Map of featureRef:feature.
*/
private Map<String, FeatureV2> getFeaturesRefToFeaturesMap(List<FeatureV2> features) {
Map<String, FeatureV2> validFeaturesMap = new HashMap<>();
for (FeatureV2 feature : features) {
ServingAPIProto.FeatureReferenceV2 featureRef =
ServingAPIProto.FeatureReferenceV2.newBuilder()
.setFeatureTable(this.getName())
.setName(feature.getName())
.build();
validFeaturesMap.put(getFeatureStringRef(featureRef), feature);
}
return validFeaturesMap;
}

/**
* Returns a list of Features if FeatureTable's Feature contains all labels in labelsFilter
*
* @param labelsFilter contain labels that should be attached to FeatureTable's features
* @return List of Features
*/
public static List<FeatureV2> filterFeaturesByAllLabels(
Set<FeatureV2> features, Map<String, String> labelsFilter) {
List<FeatureV2> validFeatures =
features.stream()
.filter(feature -> feature.hasAllLabels(labelsFilter))
.collect(Collectors.toList());

return validFeatures;
}

/**
* Determine whether a FeatureTable has all the specified labels.
*
Expand Down
17 changes: 17 additions & 0 deletions core/src/main/java/feast/core/model/FeatureV2.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,23 @@ public void updateFromProto(FeatureSpecV2 spec) {
this.labelsJSON = TypeConversion.convertMapToJsonString(spec.getLabelsMap());
}

/**
* Return a boolean to indicate if Feature contains all specified labels.
*
* @param labelsFilter contain labels that should be attached to Feature
* @return boolean True if Feature contains all labels in the labelsFilter
*/
public boolean hasAllLabels(Map<String, String> labelsFilter) {
Map<String, String> featureLabelsMap = TypeConversion.convertJsonStringToMap(getLabelsJSON());
for (String key : labelsFilter.keySet()) {
if (!featureLabelsMap.containsKey(key)
|| !featureLabelsMap.get(key).equals(labelsFilter.get(key))) {
return false;
}
}
return true;
}

@Override
public int hashCode() {
return Objects.hash(getName(), getType(), getLabelsJSON());
Expand Down
57 changes: 24 additions & 33 deletions core/src/main/java/feast/core/service/SpecService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -297,46 +297,37 @@ public ListFeatureSetsResponse listFeatureSets(ListFeatureSetsRequest.Filter fil
* filter
*/
public ListFeaturesResponse listFeatures(ListFeaturesRequest.Filter filter) {
try {
String project = filter.getProject();
List<String> entities = filter.getEntitiesList();
Map<String, String> labels = filter.getLabelsMap();
String project = filter.getProject();
List<String> entities = filter.getEntitiesList();
Map<String, String> labels = filter.getLabelsMap();

checkValidCharactersAllowAsterisk(project, "project");
checkValidCharactersAllowAsterisk(project, "project");

// Autofill default project if project not specified
if (project.isEmpty()) {
project = Project.DEFAULT_NAME;
}
// Autofill default project if project not specified
if (project.isEmpty()) {
project = Project.DEFAULT_NAME;
}

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

ListFeaturesResponse.Builder response = ListFeaturesResponse.newBuilder();
if (entities.size() > 0) {
featureSets =
featureSets.stream()
.filter(featureSet -> featureSet.hasAllEntities(entities))
.collect(Collectors.toList());
}
ListFeaturesResponse.Builder response = ListFeaturesResponse.newBuilder();
if (entities.size() > 0) {
featureTables =
featureTables.stream()
.filter(featureTable -> featureTable.hasAllEntities(entities))
.collect(Collectors.toList());
}

Map<String, Feature> featuresMap;
for (FeatureSet featureSet : featureSets) {
featuresMap = featureSet.getFeaturesByRef(labels);
for (Map.Entry<String, Feature> entry : featuresMap.entrySet()) {
response.putFeatures(entry.getKey(), entry.getValue().toProto());
}
Map<String, FeatureV2> featuresMap;
for (FeatureTable featureTable : featureTables) {
featuresMap = featureTable.getFeaturesByLabels(labels);
for (Map.Entry<String, FeatureV2> entry : featuresMap.entrySet()) {
response.putFeatures(entry.getKey(), entry.getValue().toProto());
}

return response.build();
} catch (InvalidProtocolBufferException e) {
throw io.grpc.Status.NOT_FOUND
.withDescription("Unable to retrieve features")
.withCause(e)
.asRuntimeException();
}

return response.build();
}

/**
Expand Down
18 changes: 6 additions & 12 deletions core/src/test/java/feast/core/controller/CoreServiceRestIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,12 +176,9 @@ public void listFeatureSets() {

@Test
public void listFeatures() {
// entities = [merchant_id]
// project = default
// should return 4 features
String uri1 =
UriComponentsBuilder.fromPath("/api/v1/features")
.queryParam("entities", "merchant_id")
UriComponentsBuilder.fromPath("/api/v2/features")
.queryParam("entities", "entity1", "entity2")
.buildAndExpand()
.toString();
get(uri1)
Expand All @@ -190,15 +187,12 @@ public void listFeatures() {
.everything()
.assertThat()
.contentType(ContentType.JSON)
.body("features", aMapWithSize(4));
.body("features", aMapWithSize(2));

// entities = [merchant_id]
// project = merchant
// should return 2 features
String uri2 =
UriComponentsBuilder.fromPath("/api/v1/features")
.queryParam("entities", "merchant_id")
.queryParam("project", "merchant")
UriComponentsBuilder.fromPath("/api/v2/features")
.queryParam("entities", "entity1", "entity2")
.queryParam("project", "default")
.buildAndExpand()
.toString();
get(uri2)
Expand Down
79 changes: 53 additions & 26 deletions core/src/test/java/feast/core/service/SpecServiceIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,23 @@ public void initState() {
.setBatchSource(
DataGenerator.createFileDataSourceSpec("file:///path/to/file", "ts_col", ""))
.build());
apiClient.applyFeatureTable(
"default",
DataGenerator.createFeatureTableSpec(
"featuretable2",
Arrays.asList("entity1", "entity2"),
new HashMap<>() {
{
put("feature3", ValueProto.ValueType.Enum.STRING);
put("feature4", ValueProto.ValueType.Enum.FLOAT);
}
},
7200,
ImmutableMap.of("feat_key4", "feat_value4"))
.toBuilder()
.setBatchSource(
DataGenerator.createFileDataSourceSpec("file:///path/to/file", "ts_col", ""))
.build());
apiClient.simpleApplyEntity(
"project1",
DataGenerator.createEntitySpecV2(
Expand Down Expand Up @@ -312,10 +329,13 @@ public void shouldUseDefaultProjectIfProjectUnspecified() {
List<FeatureTableProto.FeatureTable> featureTables =
apiClient.simpleListFeatureTables(filter);

assertThat(featureTables, hasSize(1));
assertThat(featureTables, hasSize(2));
assertThat(
featureTables,
hasItem(hasProperty("spec", hasProperty("name", equalTo("featuretable1")))));
assertThat(
featureTables,
hasItem(hasProperty("spec", hasProperty("name", equalTo("featuretable2")))));
}

@Test
Expand Down Expand Up @@ -1005,49 +1025,55 @@ class ListFeatures {
@Test
public void shouldFilterFeaturesByEntitiesAndLabels() {
// Case 1: Only filter by entities
Map<String, FeatureSetProto.FeatureSpec> result1 =
apiClient.simpleListFeatures("project1", "user_id");
Map<String, FeatureProto.FeatureSpecV2> result1 =
apiClient.simpleListFeatures("default", "entity1", "entity2");

assertThat(result1, aMapWithSize(2));
assertThat(result1, hasKey(equalTo("project1/fs3:feature1")));
assertThat(result1, hasKey(equalTo("project1/fs3:feature2")));
assertThat(result1, aMapWithSize(4));
assertThat(result1, hasKey(equalTo("featuretable1:feature1")));
assertThat(result1, hasKey(equalTo("featuretable1:feature2")));
assertThat(result1, hasKey(equalTo("featuretable2:feature3")));
assertThat(result1, hasKey(equalTo("featuretable2:feature4")));

// Case 2: Filter by entities and labels
Map<String, FeatureSetProto.FeatureSpec> result2 =
Map<String, FeatureProto.FeatureSpecV2> result2 =
apiClient.simpleListFeatures(
"project1",
ImmutableMap.of("app", "feast", "version", "one"),
ImmutableList.of("customer_id"));
"default",
ImmutableMap.of("feat_key2", "feat_value2"),
ImmutableList.of("entity1", "entity2"));

assertThat(result2, aMapWithSize(1));
assertThat(result2, hasKey(equalTo("project1/fs4:feature2")));
assertThat(result2, aMapWithSize(2));
assertThat(result2, hasKey(equalTo("featuretable1:feature1")));
assertThat(result2, hasKey(equalTo("featuretable1:feature2")));

// Case 3: Filter by labels
Map<String, FeatureSetProto.FeatureSpec> result3 =
Map<String, FeatureProto.FeatureSpecV2> result3 =
apiClient.simpleListFeatures(
"project1", ImmutableMap.of("app", "feast"), Collections.emptyList());
"default", ImmutableMap.of("feat_key4", "feat_value4"), Collections.emptyList());

assertThat(result3, aMapWithSize(2));
assertThat(result3, hasKey(equalTo("project1/fs4:feature2")));
assertThat(result3, hasKey(equalTo("project1/fs5:feature3")));
assertThat(result3, hasKey(equalTo("featuretable2:feature3")));
assertThat(result3, hasKey(equalTo("featuretable2:feature4")));

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

assertThat(result4, aMapWithSize(4));
assertThat(result4, hasKey(equalTo("project1/fs3:feature1")));
assertThat(result4, hasKey(equalTo("project1/fs3:feature1")));
assertThat(result4, hasKey(equalTo("project1/fs4:feature2")));
assertThat(result4, hasKey(equalTo("project1/fs5:feature3")));
assertThat(result4, aMapWithSize(0));

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

assertThat(result5, aMapWithSize(2));
assertThat(result5, hasKey(equalTo("default/fs1:total")));
assertThat(result5, hasKey(equalTo("default/fs2:sum")));
assertThat(result5, aMapWithSize(4));
assertThat(result5, hasKey(equalTo("featuretable1:feature1")));
assertThat(result5, hasKey(equalTo("featuretable1:feature2")));
assertThat(result5, hasKey(equalTo("featuretable2:feature3")));
assertThat(result5, hasKey(equalTo("featuretable2:feature4")));

// Case 6: Filter by mismatched entity
Map<String, FeatureProto.FeatureSpecV2> result6 =
apiClient.simpleListFeatures("default", ImmutableMap.of(), ImmutableList.of("entity1"));
assertThat(result6, aMapWithSize(0));
}
}

Expand Down Expand Up @@ -1350,6 +1376,7 @@ public void shouldReturnNoTables() {
CoreServiceProto.ListFeatureTablesRequest.Filter filter =
CoreServiceProto.ListFeatureTablesRequest.Filter.newBuilder()
.setProject("default")
.putLabels("feat_key2", "feat_value2")
.build();
List<FeatureTableProto.FeatureTable> featureTables =
apiClient.simpleListFeatureTables(filter);
Expand Down
Loading

0 comments on commit 77a83fd

Please sign in to comment.