Skip to content

Commit ea4e2b0

Browse files
imjuanleonardKrzysztof Suwinskisuwik
authored
Add feature and feature set labels, for metadata (#536)
* Add labels column to feature field * Add labels to feature spec proto * Implement labels on feature model * Implement list labels * Implement set_label and remove_label from feature client * Refactor ingest to only accept featureset and dtype * Add equality on labels field * Add tests for labels apply * Add Optional type hint to labels * Add empty labels key validation * Add label when generating feature spec for specServiceTest to test equality * corrected convention (push test) * corrected review comments * corrected lint-python check * corrected lint-python 2 * back out python SDK changes * Implemented labels on a feature set level * added empty keys validation * corrected review comments (storing empty json for features if labels map is empty) * Updated the comment to match the logic * added e2e tests for feature and feature set labels * moved e2e tests for feature and feature set labels * changed tests ordering Co-authored-by: Krzysztof Suwinski <krzysztof.suwinski@agoda.com> Co-authored-by: suwik <k.suwinski+git@gmail.com>
1 parent 20f491c commit ea4e2b0

File tree

14 files changed

+451
-138
lines changed

14 files changed

+451
-138
lines changed

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import feast.core.FeatureSetProto.FeatureSetSpec;
2626
import feast.core.FeatureSetProto.FeatureSetStatus;
2727
import feast.core.FeatureSetProto.FeatureSpec;
28+
import feast.core.util.TypeConversion;
2829
import feast.types.ValueProto.ValueType.Enum;
2930
import java.util.ArrayList;
3031
import java.util.HashMap;
@@ -116,6 +117,10 @@ public class FeatureSet extends AbstractTimestampEntity implements Comparable<Fe
116117
@Column(name = "status")
117118
private String status;
118119

120+
// User defined metadata
121+
@Column(name = "labels", columnDefinition = "text")
122+
private String labels;
123+
119124
public FeatureSet() {
120125
super();
121126
}
@@ -128,6 +133,7 @@ public FeatureSet(
128133
List<Field> entities,
129134
List<Field> features,
130135
Source source,
136+
Map<String, String> labels,
131137
FeatureSetStatus status) {
132138
this.maxAgeSeconds = maxAgeSeconds;
133139
this.source = source;
@@ -137,6 +143,7 @@ public FeatureSet(
137143
this.name = name;
138144
this.project = new Project(project);
139145
this.version = version;
146+
this.labels = TypeConversion.convertMapToJsonString(labels);
140147
this.setId(project, name, version);
141148
addEntities(entities);
142149
addFeatures(features);
@@ -191,6 +198,7 @@ public static FeatureSet fromProto(FeatureSetProto.FeatureSet featureSetProto) {
191198
entitySpecs,
192199
featureSpecs,
193200
source,
201+
featureSetProto.getSpec().getLabelsMap(),
194202
featureSetProto.getMeta().getStatus());
195203
}
196204

@@ -247,6 +255,7 @@ public FeatureSetProto.FeatureSet toProto() throws InvalidProtocolBufferExceptio
247255
.setMaxAge(Duration.newBuilder().setSeconds(maxAgeSeconds))
248256
.addAllEntities(entitySpecs)
249257
.addAllFeatures(featureSpecs)
258+
.putAllLabels(TypeConversion.convertJsonStringToMap(labels))
250259
.setSource(source.toProto());
251260

252261
return FeatureSetProto.FeatureSet.newBuilder().setMeta(meta).setSpec(spec).build();
@@ -352,6 +361,10 @@ private void setFeatureSpecFields(FeatureSpec.Builder featureSpecBuilder, Field
352361
featureSpecBuilder.setTimeOfDayDomain(
353362
TimeOfDayDomain.parseFrom(featureField.getTimeOfDayDomain()));
354363
}
364+
365+
if (featureField.getLabels() != null) {
366+
featureSpecBuilder.putAllLabels(featureField.getLabels());
367+
}
355368
}
356369

357370
/**

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

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,9 @@
1818

1919
import feast.core.FeatureSetProto.EntitySpec;
2020
import feast.core.FeatureSetProto.FeatureSpec;
21-
import feast.types.ValueProto.ValueType;
21+
import feast.core.util.TypeConversion;
2222
import java.util.Arrays;
23+
import java.util.Map;
2324
import java.util.Objects;
2425
import javax.persistence.Column;
2526
import javax.persistence.Embeddable;
@@ -47,6 +48,10 @@ public class Field {
4748
@Column(name = "project")
4849
private String project;
4950

51+
// Labels that this field belongs to
52+
@Column(name = "labels", columnDefinition = "text")
53+
private String labels;
54+
5055
// Presence constraints (refer to proto feast.core.FeatureSet.FeatureSpec)
5156
// Only one of them can be set.
5257
private byte[] presence;
@@ -74,14 +79,10 @@ public class Field {
7479

7580
public Field() {}
7681

77-
public Field(String name, ValueType.Enum type) {
78-
this.name = name;
79-
this.type = type.toString();
80-
}
81-
8282
public Field(FeatureSpec featureSpec) {
8383
this.name = featureSpec.getName();
8484
this.type = featureSpec.getValueType().toString();
85+
this.labels = TypeConversion.convertMapToJsonString(featureSpec.getLabelsMap());
8586

8687
switch (featureSpec.getPresenceConstraintsCase()) {
8788
case PRESENCE:
@@ -215,6 +216,10 @@ public Field(EntitySpec entitySpec) {
215216
}
216217
}
217218

219+
public Map<String, String> getLabels() {
220+
return TypeConversion.convertJsonStringToMap(this.labels);
221+
}
222+
218223
@Override
219224
public boolean equals(Object o) {
220225
if (this == o) {
@@ -227,6 +232,7 @@ public boolean equals(Object o) {
227232
return Objects.equals(name, field.name)
228233
&& Objects.equals(type, field.type)
229234
&& Objects.equals(project, field.project)
235+
&& Objects.equals(labels, field.labels)
230236
&& Arrays.equals(presence, field.presence)
231237
&& Arrays.equals(groupPresence, field.groupPresence)
232238
&& Arrays.equals(shape, field.shape)
@@ -247,6 +253,6 @@ public boolean equals(Object o) {
247253

248254
@Override
249255
public int hashCode() {
250-
return Objects.hash(super.hashCode(), name, type);
256+
return Objects.hash(super.hashCode(), name, type, project, labels);
251257
}
252258
}

‎core/src/main/java/feast/core/util/TypeConversion.java‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,6 @@ public static Map<String, String> convertJsonStringToMap(String jsonString) {
7070
* @return json string corresponding to given map
7171
*/
7272
public static String convertMapToJsonString(Map<String, String> map) {
73-
if (map.isEmpty()) {
74-
return "{}";
75-
}
7673
return gson.toJson(map);
7774
}
7875

‎core/src/main/java/feast/core/validators/FeatureSetValidator.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,17 @@
2727
import java.util.stream.Collectors;
2828

2929
public class FeatureSetValidator {
30+
3031
public static void validateSpec(FeatureSet featureSet) {
3132
if (featureSet.getSpec().getProject().isEmpty()) {
3233
throw new IllegalArgumentException("Project name must be provided");
3334
}
3435
if (featureSet.getSpec().getName().isEmpty()) {
3536
throw new IllegalArgumentException("Feature set name must be provided");
3637
}
38+
if (featureSet.getSpec().getLabelsMap().containsKey("")) {
39+
throw new IllegalArgumentException("Feature set label keys must not be empty");
40+
}
3741

3842
checkValidCharacters(featureSet.getSpec().getProject(), "project");
3943
checkValidCharacters(featureSet.getSpec().getName(), "name");
@@ -44,6 +48,9 @@ public static void validateSpec(FeatureSet featureSet) {
4448
}
4549
for (FeatureSpec featureSpec : featureSet.getSpec().getFeaturesList()) {
4650
checkValidCharacters(featureSpec.getName(), "features::name");
51+
if (featureSpec.getLabelsMap().containsKey("")) {
52+
throw new IllegalArgumentException("Feature label keys must not be empty");
53+
}
4754
}
4855
}
4956

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

Lines changed: 5 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,8 @@
3434
import feast.core.CoreServiceProto.RestartIngestionJobResponse;
3535
import feast.core.CoreServiceProto.StopIngestionJobRequest;
3636
import feast.core.CoreServiceProto.StopIngestionJobResponse;
37-
import feast.core.FeatureSetProto.FeatureSetStatus;
3837
import feast.core.FeatureSetReferenceProto.FeatureSetReference;
3938
import feast.core.IngestionJobProto.IngestionJob;
40-
import feast.core.SourceProto.KafkaSourceConfig;
41-
import feast.core.SourceProto.SourceType;
4239
import feast.core.StoreProto.Store.RedisConfig;
4340
import feast.core.StoreProto.Store.StoreType;
4441
import feast.core.dao.JobRepository;
@@ -84,14 +81,7 @@ public void setup() {
8481

8582
// create mock objects for testing
8683
// fake data source
87-
this.dataSource =
88-
new Source(
89-
SourceType.KAFKA,
90-
KafkaSourceConfig.newBuilder()
91-
.setBootstrapServers("kafka:9092")
92-
.setTopic("my-topic")
93-
.build(),
94-
true);
84+
this.dataSource = TestObjectFactory.defaultSource;
9585
// fake data store
9686
this.dataStore =
9787
new Store(
@@ -158,19 +148,12 @@ public void setupJobManager() {
158148

159149
// dummy model constructorss
160150
private FeatureSet newDummyFeatureSet(String name, int version, String project) {
161-
Field feature = new Field(name + "_feature", Enum.INT64);
162-
Field entity = new Field(name + "_entity", Enum.STRING);
151+
Field feature = TestObjectFactory.CreateFeatureField(name + "_feature", Enum.INT64);
152+
Field entity = TestObjectFactory.CreateEntityField(name + "_entity", Enum.STRING);
163153

164154
FeatureSet fs =
165-
new FeatureSet(
166-
name,
167-
project,
168-
version,
169-
100L,
170-
Arrays.asList(entity),
171-
Arrays.asList(feature),
172-
this.dataSource,
173-
FeatureSetStatus.STATUS_READY);
155+
TestObjectFactory.CreateFeatureSet(
156+
name, project, version, Arrays.asList(entity), Arrays.asList(feature));
174157
fs.setCreated(Date.from(Instant.ofEpochSecond(10L)));
175158
return fs;
176159
}

0 commit comments

Comments
 (0)