From 3c08158e51d37b5c1ad15a39318df17f7192a5f3 Mon Sep 17 00:00:00 2001 From: zhilingc Date: Mon, 27 Apr 2020 00:25:21 +0800 Subject: [PATCH 01/11] Split Field model into distinct Feature and Entity objects --- .../feast/core/dao/MetricsRepository.java | 27 -- .../main/java/feast/core/model/Entity.java | 69 +++++ .../feast/core/model/EntityReference.java | 72 +++++ .../main/java/feast/core/model/Feature.java | 180 ++++++++++++ .../feast/core/model/FeatureReference.java | 72 +++++ .../java/feast/core/model/FeatureSet.java | 193 ++++--------- .../src/main/java/feast/core/model/Field.java | 258 ------------------ core/src/main/java/feast/core/model/Job.java | 11 - .../main/java/feast/core/model/Metrics.java | 64 ----- .../feast/core/service/JobServiceTest.java | 6 +- .../feast/core/service/SpecServiceTest.java | 7 +- protos/feast/core/FeatureSet.proto | 40 --- sdk/python/feast/entity.py | 24 +- sdk/python/feast/feature_set.py | 2 + sdk/python/tests/test_feature_set.py | 3 - 15 files changed, 458 insertions(+), 570 deletions(-) delete mode 100644 core/src/main/java/feast/core/dao/MetricsRepository.java create mode 100644 core/src/main/java/feast/core/model/Entity.java create mode 100644 core/src/main/java/feast/core/model/EntityReference.java create mode 100644 core/src/main/java/feast/core/model/Feature.java create mode 100644 core/src/main/java/feast/core/model/FeatureReference.java delete mode 100644 core/src/main/java/feast/core/model/Field.java delete mode 100644 core/src/main/java/feast/core/model/Metrics.java diff --git a/core/src/main/java/feast/core/dao/MetricsRepository.java b/core/src/main/java/feast/core/dao/MetricsRepository.java deleted file mode 100644 index 7146e1e3ecb..00000000000 --- a/core/src/main/java/feast/core/dao/MetricsRepository.java +++ /dev/null @@ -1,27 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * Copyright 2018-2019 The Feast Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package feast.core.dao; - -import feast.core.model.Metrics; -import java.util.List; -import org.springframework.data.jpa.repository.JpaRepository; -import org.springframework.stereotype.Repository; - -@Repository -public interface MetricsRepository extends JpaRepository { - List findByJob_Id(String id); -} diff --git a/core/src/main/java/feast/core/model/Entity.java b/core/src/main/java/feast/core/model/Entity.java new file mode 100644 index 00000000000..b9d2a8827f2 --- /dev/null +++ b/core/src/main/java/feast/core/model/Entity.java @@ -0,0 +1,69 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright 2018-2019 The Feast Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package feast.core.model; + +import feast.core.FeatureSetProto.EntitySpec; +import feast.types.ValueProto.ValueType; +import java.util.Objects; +import javax.persistence.EmbeddedId; +import lombok.Getter; +import lombok.Setter; + +/** Feast entity object. Contains name, type as well as domain metadata about the entity. */ +@Getter +@Setter +@javax.persistence.Entity +public class Entity { + @EmbeddedId private EntityReference reference; + + private String type; + + public Entity() {} + + private Entity(String name, ValueType.Enum type) { + this.setReference(new EntityReference(name)); + this.setType(type.toString()); + } + + public static Entity withRef(EntityReference entityRef) { + Entity entity = new Entity(); + entity.setReference(entityRef); + return entity; + } + + public static Entity fromProto(EntitySpec entitySpec) { + Entity entity = new Entity(entitySpec.getName(), entitySpec.getValueType()); + return entity; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + Entity feature = (Entity) o; + return getReference().equals(feature.getReference()) && getType().equals(feature.getType()); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), getReference(), getType()); + } +} diff --git a/core/src/main/java/feast/core/model/EntityReference.java b/core/src/main/java/feast/core/model/EntityReference.java new file mode 100644 index 00000000000..88d808df7fc --- /dev/null +++ b/core/src/main/java/feast/core/model/EntityReference.java @@ -0,0 +1,72 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright 2018-2020 The Feast Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package feast.core.model; + +import java.io.Serializable; +import java.util.Objects; +import javax.persistence.Column; +import javax.persistence.Embeddable; +import lombok.AllArgsConstructor; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.Setter; + +@Embeddable +@NoArgsConstructor +@AllArgsConstructor +@Getter +@Setter +public class EntityReference implements Serializable { + // Project the field belongs to + @Column(nullable = false) + private String project; + + // Feature set the field belongs to + @Column(name = "feature_set", nullable = false) + private String featureSet; + + // Version of the feature set this field belongs to + @Column(nullable = false) + private int version; + + // Name of the field + @Column(nullable = false) + private String name; + + EntityReference(String name) { + this.name = name; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + EntityReference fieldId = (EntityReference) o; + return Objects.equals(name, fieldId.getName()) + && Objects.equals(project, fieldId.getProject()) + && Objects.equals(featureSet, fieldId.getFeatureSet()); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), project, featureSet, name); + } +} diff --git a/core/src/main/java/feast/core/model/Feature.java b/core/src/main/java/feast/core/model/Feature.java new file mode 100644 index 00000000000..907e3446a1b --- /dev/null +++ b/core/src/main/java/feast/core/model/Feature.java @@ -0,0 +1,180 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright 2018-2019 The Feast Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package feast.core.model; + +import feast.core.FeatureSetProto.FeatureSpec; +import feast.types.ValueProto.ValueType; +import java.util.Arrays; +import java.util.Objects; +import javax.persistence.*; +import javax.persistence.Entity; +import lombok.Getter; +import lombok.Setter; + +/** + * Feature belonging to a featureset. Contains name, type as well as domain metadata about the + * feature. + */ +@Getter +@Setter +@Entity +public class Feature { + + @EmbeddedId private FeatureReference reference; + + // Type of the field + private String type; + + // Presence constraints (refer to proto feast.core.FeatureSet.FeatureSpec) + // Only one of them can be set. + private byte[] presence; + private byte[] groupPresence; + + // Shape type (refer to proto feast.core.FeatureSet.FeatureSpec) + // Only one of them can be set. + private byte[] shape; + private byte[] valueCount; + + // Domain info for the values (refer to proto feast.core.FeatureSet.FeatureSpec) + // Only one of them can be set. + private String domain; + private byte[] intDomain; + private byte[] floatDomain; + private byte[] stringDomain; + private byte[] boolDomain; + private byte[] structDomain; + private byte[] naturalLanguageDomain; + private byte[] imageDomain; + private byte[] midDomain; + private byte[] urlDomain; + private byte[] timeDomain; + private byte[] timeOfDayDomain; + + private Feature() {} + + private Feature(String name, ValueType.Enum type) { + this.setReference(new FeatureReference(name)); + this.setType(type.toString()); + } + + public static Feature withReference(FeatureReference featureRef) { + Feature feature = new Feature(); + feature.setReference(featureRef); + return feature; + } + + public static Feature fromProto(FeatureSpec featureSpec) { + Feature feature = new Feature(featureSpec.getName(), featureSpec.getValueType()); + + switch (featureSpec.getPresenceConstraintsCase()) { + case PRESENCE: + feature.setPresence(featureSpec.getPresence().toByteArray()); + break; + case GROUP_PRESENCE: + feature.setGroupPresence(featureSpec.getGroupPresence().toByteArray()); + break; + case PRESENCECONSTRAINTS_NOT_SET: + break; + } + + switch (featureSpec.getShapeTypeCase()) { + case SHAPE: + feature.setShape(featureSpec.getShape().toByteArray()); + break; + case VALUE_COUNT: + feature.setValueCount(featureSpec.getValueCount().toByteArray()); + break; + case SHAPETYPE_NOT_SET: + break; + } + + switch (featureSpec.getDomainInfoCase()) { + case DOMAIN: + feature.setDomain(featureSpec.getDomain()); + break; + case INT_DOMAIN: + feature.setIntDomain(featureSpec.getIntDomain().toByteArray()); + break; + case FLOAT_DOMAIN: + feature.setFloatDomain(featureSpec.getFloatDomain().toByteArray()); + break; + case STRING_DOMAIN: + feature.setStringDomain(featureSpec.getStringDomain().toByteArray()); + break; + case BOOL_DOMAIN: + feature.setBoolDomain(featureSpec.getBoolDomain().toByteArray()); + break; + case STRUCT_DOMAIN: + feature.setStructDomain(featureSpec.getStructDomain().toByteArray()); + break; + case NATURAL_LANGUAGE_DOMAIN: + feature.setNaturalLanguageDomain(featureSpec.getNaturalLanguageDomain().toByteArray()); + break; + case IMAGE_DOMAIN: + feature.setImageDomain(featureSpec.getImageDomain().toByteArray()); + break; + case MID_DOMAIN: + feature.setMidDomain(featureSpec.getMidDomain().toByteArray()); + break; + case URL_DOMAIN: + feature.setUrlDomain(featureSpec.getUrlDomain().toByteArray()); + break; + case TIME_DOMAIN: + feature.setTimeDomain(featureSpec.getTimeDomain().toByteArray()); + break; + case TIME_OF_DAY_DOMAIN: + feature.setTimeOfDayDomain(featureSpec.getTimeOfDayDomain().toByteArray()); + break; + case DOMAININFO_NOT_SET: + break; + } + return feature; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + Feature feature = (Feature) o; + return Objects.equals(getReference(), feature.getReference()) + && Arrays.equals(getPresence(), feature.getPresence()) + && Arrays.equals(getGroupPresence(), feature.getGroupPresence()) + && Arrays.equals(getShape(), feature.getShape()) + && Arrays.equals(getValueCount(), feature.getValueCount()) + && Objects.equals(getDomain(), feature.getDomain()) + && Arrays.equals(getIntDomain(), feature.getIntDomain()) + && Arrays.equals(getFloatDomain(), feature.getFloatDomain()) + && Arrays.equals(getStringDomain(), feature.getStringDomain()) + && Arrays.equals(getBoolDomain(), feature.getBoolDomain()) + && Arrays.equals(getStructDomain(), feature.getStructDomain()) + && Arrays.equals(getNaturalLanguageDomain(), feature.getNaturalLanguageDomain()) + && Arrays.equals(getImageDomain(), feature.getImageDomain()) + && Arrays.equals(getMidDomain(), feature.getMidDomain()) + && Arrays.equals(getUrlDomain(), feature.getUrlDomain()) + && Arrays.equals(getTimeDomain(), feature.getTimeDomain()) + && Arrays.equals(getTimeDomain(), feature.getTimeOfDayDomain()); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), getReference(), getType()); + } +} diff --git a/core/src/main/java/feast/core/model/FeatureReference.java b/core/src/main/java/feast/core/model/FeatureReference.java new file mode 100644 index 00000000000..b27c17f1b94 --- /dev/null +++ b/core/src/main/java/feast/core/model/FeatureReference.java @@ -0,0 +1,72 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright 2018-2020 The Feast Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package feast.core.model; + +import java.io.Serializable; +import java.util.Objects; +import javax.persistence.Column; +import javax.persistence.Embeddable; +import lombok.AllArgsConstructor; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.Setter; + +@Embeddable +@NoArgsConstructor +@AllArgsConstructor +@Getter +@Setter +public class FeatureReference implements Serializable { + // Project the field belongs to + @Column(nullable = false) + private String project; + + // Feature set the field belongs to + @Column(name = "feature_set", nullable = false) + private String featureSet; + + // Version of the feature set this field belongs to + @Column(nullable = false) + private int version; + + // Name of the field + @Column(nullable = false) + private String name; + + FeatureReference(String name) { + this.name = name; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + FeatureReference fieldId = (FeatureReference) o; + return Objects.equals(name, fieldId.getName()) + && Objects.equals(project, fieldId.getProject()) + && Objects.equals(featureSet, fieldId.getFeatureSet()); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), project, featureSet, name); + } +} diff --git a/core/src/main/java/feast/core/model/FeatureSet.java b/core/src/main/java/feast/core/model/FeatureSet.java index ec8da77c5f9..71cae52737d 100644 --- a/core/src/main/java/feast/core/model/FeatureSet.java +++ b/core/src/main/java/feast/core/model/FeatureSet.java @@ -20,54 +20,18 @@ import com.google.protobuf.InvalidProtocolBufferException; import com.google.protobuf.Timestamp; import feast.core.FeatureSetProto; -import feast.core.FeatureSetProto.EntitySpec; -import feast.core.FeatureSetProto.FeatureSetMeta; -import feast.core.FeatureSetProto.FeatureSetSpec; -import feast.core.FeatureSetProto.FeatureSetStatus; -import feast.core.FeatureSetProto.FeatureSpec; -import feast.core.util.TypeConversion; +import feast.core.FeatureSetProto.*; import feast.types.ValueProto.ValueType.Enum; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; -import javax.persistence.CascadeType; -import javax.persistence.CollectionTable; -import javax.persistence.Column; -import javax.persistence.ElementCollection; -import javax.persistence.Entity; -import javax.persistence.FetchType; -import javax.persistence.Id; -import javax.persistence.JoinColumn; -import javax.persistence.ManyToOne; -import javax.persistence.Table; -import javax.persistence.UniqueConstraint; +import java.util.*; +import javax.persistence.*; import lombok.Getter; import lombok.Setter; import org.apache.commons.lang3.builder.HashCodeBuilder; -import org.hibernate.annotations.Fetch; -import org.hibernate.annotations.FetchMode; -import org.tensorflow.metadata.v0.BoolDomain; -import org.tensorflow.metadata.v0.FeaturePresence; -import org.tensorflow.metadata.v0.FeaturePresenceWithinGroup; -import org.tensorflow.metadata.v0.FixedShape; -import org.tensorflow.metadata.v0.FloatDomain; -import org.tensorflow.metadata.v0.ImageDomain; -import org.tensorflow.metadata.v0.IntDomain; -import org.tensorflow.metadata.v0.MIDDomain; -import org.tensorflow.metadata.v0.NaturalLanguageDomain; -import org.tensorflow.metadata.v0.StringDomain; -import org.tensorflow.metadata.v0.StructDomain; -import org.tensorflow.metadata.v0.TimeDomain; -import org.tensorflow.metadata.v0.TimeOfDayDomain; -import org.tensorflow.metadata.v0.URLDomain; -import org.tensorflow.metadata.v0.ValueCount; +import org.tensorflow.metadata.v0.*; @Getter @Setter -@Entity +@javax.persistence.Entity @Table(name = "feature_sets") public class FeatureSet extends AbstractTimestampEntity implements Comparable { @@ -94,19 +58,12 @@ public class FeatureSet extends AbstractTimestampEntity implements Comparable entities; + @OneToMany(cascade = CascadeType.ALL, fetch = FetchType.EAGER) + private Set entities; // Feature fields inside this feature set - @ElementCollection(fetch = FetchType.EAGER) - @CollectionTable( - name = "features", - joinColumns = @JoinColumn(name = "feature_set_id"), - uniqueConstraints = @UniqueConstraint(columnNames = {"name", "project", "version"})) - @Fetch(FetchMode.SUBSELECT) - private Set features; + @OneToMany(cascade = CascadeType.ALL, fetch = FetchType.EAGER) + private Set features; // Source on which feature rows can be found @ManyToOne(cascade = CascadeType.ALL, fetch = FetchType.EAGER) @@ -130,8 +87,8 @@ public FeatureSet( String project, int version, long maxAgeSeconds, - List entities, - List features, + List entities, + List features, Source source, Map labels, FeatureSetStatus status) { @@ -180,14 +137,14 @@ public static FeatureSet fromProto(FeatureSetProto.FeatureSet featureSetProto) { FeatureSetSpec featureSetSpec = featureSetProto.getSpec(); Source source = Source.fromProto(featureSetSpec.getSource()); - List featureSpecs = new ArrayList<>(); + List featureSpecs = new ArrayList<>(); for (FeatureSpec featureSpec : featureSetSpec.getFeaturesList()) { - featureSpecs.add(new Field(featureSpec)); + featureSpecs.add(Feature.fromProto(featureSpec)); } - List entitySpecs = new ArrayList<>(); + List entitySpecs = new ArrayList<>(); for (EntitySpec entitySpec : featureSetSpec.getEntitiesList()) { - entitySpecs.add(new Field(entitySpec)); + entitySpecs.add(Entity.fromProto(entitySpec)); } return new FeatureSet( @@ -202,40 +159,44 @@ public static FeatureSet fromProto(FeatureSetProto.FeatureSet featureSetProto) { featureSetProto.getMeta().getStatus()); } - public void addEntities(List fields) { - for (Field field : fields) { - addEntity(field); + public void addEntities(List entities) { + for (Entity entity : entities) { + addEntity(entity); } } - public void addEntity(Field field) { - field.setProject(this.project.getName()); - field.setVersion(this.getVersion()); - entities.add(field); + public void addEntity(Entity entity) { + EntityReference entityReference = entity.getReference(); + entityReference.setProject(this.project.getName()); + entityReference.setFeatureSet(this.getName()); + entityReference.setVersion(this.getVersion()); + entities.add(entity); } - public void addFeatures(List fields) { - for (Field field : fields) { - addFeature(field); + public void addFeatures(List features) { + for (Feature feature : features) { + addFeature(feature); } } - public void addFeature(Field field) { - field.setProject(this.project.getName()); - field.setVersion(this.getVersion()); - features.add(field); + public void addFeature(Feature feature) { + FeatureReference featureReference = feature.getReference(); + featureReference.setProject(this.project.getName()); + featureReference.setFeatureSet(this.getName()); + featureReference.setVersion(this.getVersion()); + features.add(feature); } public FeatureSetProto.FeatureSet toProto() throws InvalidProtocolBufferException { List entitySpecs = new ArrayList<>(); - for (Field entityField : entities) { + for (Entity entityField : entities) { EntitySpec.Builder entitySpecBuilder = EntitySpec.newBuilder(); setEntitySpecFields(entitySpecBuilder, entityField); entitySpecs.add(entitySpecBuilder.build()); } List featureSpecs = new ArrayList<>(); - for (Field featureField : features) { + for (Feature featureField : features) { FeatureSpec.Builder featureSpecBuilder = FeatureSpec.newBuilder(); setFeatureSpecFields(featureSpecBuilder, featureField); featureSpecs.add(featureSpecBuilder.build()); @@ -261,64 +222,16 @@ public FeatureSetProto.FeatureSet toProto() throws InvalidProtocolBufferExceptio return FeatureSetProto.FeatureSet.newBuilder().setMeta(meta).setSpec(spec).build(); } - // setEntitySpecFields and setFeatureSpecFields methods contain duplicated code because - // Feast internally treat EntitySpec and FeatureSpec as Field class. However, the proto message - // builder for EntitySpec and FeatureSpec are of different class. - @SuppressWarnings("DuplicatedCode") - private void setEntitySpecFields(EntitySpec.Builder entitySpecBuilder, Field entityField) - throws InvalidProtocolBufferException { + private void setEntitySpecFields(EntitySpec.Builder entitySpecBuilder, Entity entityField) { entitySpecBuilder - .setName(entityField.getName()) + .setName(entityField.getReference().getName()) .setValueType(Enum.valueOf(entityField.getType())); - - if (entityField.getPresence() != null) { - entitySpecBuilder.setPresence(FeaturePresence.parseFrom(entityField.getPresence())); - } else if (entityField.getGroupPresence() != null) { - entitySpecBuilder.setGroupPresence( - FeaturePresenceWithinGroup.parseFrom(entityField.getGroupPresence())); - } - - if (entityField.getShape() != null) { - entitySpecBuilder.setShape(FixedShape.parseFrom(entityField.getShape())); - } else if (entityField.getValueCount() != null) { - entitySpecBuilder.setValueCount(ValueCount.parseFrom(entityField.getValueCount())); - } - - if (entityField.getDomain() != null) { - entitySpecBuilder.setDomain(entityField.getDomain()); - } else if (entityField.getIntDomain() != null) { - entitySpecBuilder.setIntDomain(IntDomain.parseFrom(entityField.getIntDomain())); - } else if (entityField.getFloatDomain() != null) { - entitySpecBuilder.setFloatDomain(FloatDomain.parseFrom(entityField.getFloatDomain())); - } else if (entityField.getStringDomain() != null) { - entitySpecBuilder.setStringDomain(StringDomain.parseFrom(entityField.getStringDomain())); - } else if (entityField.getBoolDomain() != null) { - entitySpecBuilder.setBoolDomain(BoolDomain.parseFrom(entityField.getBoolDomain())); - } else if (entityField.getStructDomain() != null) { - entitySpecBuilder.setStructDomain(StructDomain.parseFrom(entityField.getStructDomain())); - } else if (entityField.getNaturalLanguageDomain() != null) { - entitySpecBuilder.setNaturalLanguageDomain( - NaturalLanguageDomain.parseFrom(entityField.getNaturalLanguageDomain())); - } else if (entityField.getImageDomain() != null) { - entitySpecBuilder.setImageDomain(ImageDomain.parseFrom(entityField.getImageDomain())); - } else if (entityField.getMidDomain() != null) { - entitySpecBuilder.setIntDomain(IntDomain.parseFrom(entityField.getIntDomain())); - } else if (entityField.getUrlDomain() != null) { - entitySpecBuilder.setUrlDomain(URLDomain.parseFrom(entityField.getUrlDomain())); - } else if (entityField.getTimeDomain() != null) { - entitySpecBuilder.setTimeDomain(TimeDomain.parseFrom(entityField.getTimeDomain())); - } else if (entityField.getTimeOfDayDomain() != null) { - entitySpecBuilder.setTimeOfDayDomain( - TimeOfDayDomain.parseFrom(entityField.getTimeOfDayDomain())); - } } - // Refer to setEntitySpecFields method for the reason for code duplication. - @SuppressWarnings("DuplicatedCode") - private void setFeatureSpecFields(FeatureSpec.Builder featureSpecBuilder, Field featureField) + private void setFeatureSpecFields(FeatureSpec.Builder featureSpecBuilder, Feature featureField) throws InvalidProtocolBufferException { featureSpecBuilder - .setName(featureField.getName()) + .setName(featureField.getReference().getName()) .setValueType(Enum.valueOf(featureField.getType())); if (featureField.getPresence() != null) { @@ -391,36 +304,40 @@ public boolean equalTo(FeatureSet other) { } // Create a map of all fields in this feature set - Map fields = new HashMap<>(); + Map entitiesMap = new HashMap<>(); + Map featuresMap = new HashMap<>(); - for (Field e : entities) { - fields.putIfAbsent(e.getName(), e); + for (Entity e : entities) { + entitiesMap.putIfAbsent(e.getReference().getName(), e); } - for (Field f : features) { - fields.putIfAbsent(f.getName(), f); + for (Feature f : features) { + featuresMap.putIfAbsent(f.getReference().getName(), f); } // Ensure map size is consistent with existing fields - if (fields.size() != other.getFeatures().size() + other.getEntities().size()) { + if (entitiesMap.size() != other.getEntities().size()) { + return false; + } + if (featuresMap.size() != other.getFeatures().size()) { return false; } // Ensure the other entities and features exist in the field map - for (Field e : other.getEntities()) { - if (!fields.containsKey(e.getName())) { + for (Entity e : other.getEntities()) { + if (!entitiesMap.containsKey(e.getReference().getName())) { return false; } - if (!e.equals(fields.get(e.getName()))) { + if (!e.equals(entitiesMap.get(e.getReference().getName()))) { return false; } } - for (Field f : other.getFeatures()) { - if (!fields.containsKey(f.getName())) { + for (Feature f : other.getFeatures()) { + if (!featuresMap.containsKey(f.getReference().getName())) { return false; } - if (!f.equals(fields.get(f.getName()))) { + if (!f.equals(featuresMap.get(f.getReference().getName()))) { return false; } } diff --git a/core/src/main/java/feast/core/model/Field.java b/core/src/main/java/feast/core/model/Field.java deleted file mode 100644 index 213c17f954a..00000000000 --- a/core/src/main/java/feast/core/model/Field.java +++ /dev/null @@ -1,258 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * Copyright 2018-2019 The Feast Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package feast.core.model; - -import feast.core.FeatureSetProto.EntitySpec; -import feast.core.FeatureSetProto.FeatureSpec; -import feast.core.util.TypeConversion; -import java.util.Arrays; -import java.util.Map; -import java.util.Objects; -import javax.persistence.Column; -import javax.persistence.Embeddable; -import lombok.Getter; -import lombok.Setter; - -@Getter -@Setter -@Embeddable -public class Field { - - // Name of the feature - @Column(name = "name", nullable = false) - private String name; - - // Type of the feature, should correspond with feast.types.ValueType - @Column(name = "type", nullable = false) - private String type; - - // Version of the field - @Column(name = "version") - private int version; - - // Project that this field belongs to - @Column(name = "project") - private String project; - - // Labels that this field belongs to - @Column(name = "labels", columnDefinition = "text") - private String labels; - - // Presence constraints (refer to proto feast.core.FeatureSet.FeatureSpec) - // Only one of them can be set. - private byte[] presence; - private byte[] groupPresence; - - // Shape type (refer to proto feast.core.FeatureSet.FeatureSpec) - // Only one of them can be set. - private byte[] shape; - private byte[] valueCount; - - // Domain info for the values (refer to proto feast.core.FeatureSet.FeatureSpec) - // Only one of them can be set. - private String domain; - private byte[] intDomain; - private byte[] floatDomain; - private byte[] stringDomain; - private byte[] boolDomain; - private byte[] structDomain; - private byte[] naturalLanguageDomain; - private byte[] imageDomain; - private byte[] midDomain; - private byte[] urlDomain; - private byte[] timeDomain; - private byte[] timeOfDayDomain; - - public Field() {} - - public Field(FeatureSpec featureSpec) { - this.name = featureSpec.getName(); - this.type = featureSpec.getValueType().toString(); - this.labels = TypeConversion.convertMapToJsonString(featureSpec.getLabelsMap()); - - switch (featureSpec.getPresenceConstraintsCase()) { - case PRESENCE: - this.presence = featureSpec.getPresence().toByteArray(); - break; - case GROUP_PRESENCE: - this.groupPresence = featureSpec.getGroupPresence().toByteArray(); - break; - case PRESENCECONSTRAINTS_NOT_SET: - break; - } - - switch (featureSpec.getShapeTypeCase()) { - case SHAPE: - this.shape = featureSpec.getShape().toByteArray(); - break; - case VALUE_COUNT: - this.valueCount = featureSpec.getValueCount().toByteArray(); - break; - case SHAPETYPE_NOT_SET: - break; - } - - switch (featureSpec.getDomainInfoCase()) { - case DOMAIN: - this.domain = featureSpec.getDomain(); - break; - case INT_DOMAIN: - this.intDomain = featureSpec.getIntDomain().toByteArray(); - break; - case FLOAT_DOMAIN: - this.floatDomain = featureSpec.getFloatDomain().toByteArray(); - break; - case STRING_DOMAIN: - this.stringDomain = featureSpec.getStringDomain().toByteArray(); - break; - case BOOL_DOMAIN: - this.boolDomain = featureSpec.getBoolDomain().toByteArray(); - break; - case STRUCT_DOMAIN: - this.structDomain = featureSpec.getStructDomain().toByteArray(); - break; - case NATURAL_LANGUAGE_DOMAIN: - this.naturalLanguageDomain = featureSpec.getNaturalLanguageDomain().toByteArray(); - break; - case IMAGE_DOMAIN: - this.imageDomain = featureSpec.getImageDomain().toByteArray(); - break; - case MID_DOMAIN: - this.midDomain = featureSpec.getMidDomain().toByteArray(); - break; - case URL_DOMAIN: - this.urlDomain = featureSpec.getUrlDomain().toByteArray(); - break; - case TIME_DOMAIN: - this.timeDomain = featureSpec.getTimeDomain().toByteArray(); - break; - case TIME_OF_DAY_DOMAIN: - this.timeOfDayDomain = featureSpec.getTimeOfDayDomain().toByteArray(); - break; - case DOMAININFO_NOT_SET: - break; - } - } - - public Field(EntitySpec entitySpec) { - this.name = entitySpec.getName(); - this.type = entitySpec.getValueType().toString(); - - switch (entitySpec.getPresenceConstraintsCase()) { - case PRESENCE: - this.presence = entitySpec.getPresence().toByteArray(); - break; - case GROUP_PRESENCE: - this.groupPresence = entitySpec.getGroupPresence().toByteArray(); - break; - case PRESENCECONSTRAINTS_NOT_SET: - break; - } - - switch (entitySpec.getShapeTypeCase()) { - case SHAPE: - this.shape = entitySpec.getShape().toByteArray(); - break; - case VALUE_COUNT: - this.valueCount = entitySpec.getValueCount().toByteArray(); - break; - case SHAPETYPE_NOT_SET: - break; - } - - switch (entitySpec.getDomainInfoCase()) { - case DOMAIN: - this.domain = entitySpec.getDomain(); - break; - case INT_DOMAIN: - this.intDomain = entitySpec.getIntDomain().toByteArray(); - break; - case FLOAT_DOMAIN: - this.floatDomain = entitySpec.getFloatDomain().toByteArray(); - break; - case STRING_DOMAIN: - this.stringDomain = entitySpec.getStringDomain().toByteArray(); - break; - case BOOL_DOMAIN: - this.boolDomain = entitySpec.getBoolDomain().toByteArray(); - break; - case STRUCT_DOMAIN: - this.structDomain = entitySpec.getStructDomain().toByteArray(); - break; - case NATURAL_LANGUAGE_DOMAIN: - this.naturalLanguageDomain = entitySpec.getNaturalLanguageDomain().toByteArray(); - break; - case IMAGE_DOMAIN: - this.imageDomain = entitySpec.getImageDomain().toByteArray(); - break; - case MID_DOMAIN: - this.midDomain = entitySpec.getMidDomain().toByteArray(); - break; - case URL_DOMAIN: - this.urlDomain = entitySpec.getUrlDomain().toByteArray(); - break; - case TIME_DOMAIN: - this.timeDomain = entitySpec.getTimeDomain().toByteArray(); - break; - case TIME_OF_DAY_DOMAIN: - this.timeOfDayDomain = entitySpec.getTimeOfDayDomain().toByteArray(); - break; - case DOMAININFO_NOT_SET: - break; - } - } - - public Map getLabels() { - return TypeConversion.convertJsonStringToMap(this.labels); - } - - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - Field field = (Field) o; - return Objects.equals(name, field.name) - && Objects.equals(type, field.type) - && Objects.equals(project, field.project) - && Objects.equals(labels, field.labels) - && Arrays.equals(presence, field.presence) - && Arrays.equals(groupPresence, field.groupPresence) - && Arrays.equals(shape, field.shape) - && Arrays.equals(valueCount, field.valueCount) - && Objects.equals(domain, field.domain) - && Arrays.equals(intDomain, field.intDomain) - && Arrays.equals(floatDomain, field.floatDomain) - && Arrays.equals(stringDomain, field.stringDomain) - && Arrays.equals(boolDomain, field.boolDomain) - && Arrays.equals(structDomain, field.structDomain) - && Arrays.equals(naturalLanguageDomain, field.naturalLanguageDomain) - && Arrays.equals(imageDomain, field.imageDomain) - && Arrays.equals(midDomain, field.midDomain) - && Arrays.equals(urlDomain, field.urlDomain) - && Arrays.equals(timeDomain, field.timeDomain) - && Arrays.equals(timeOfDayDomain, field.timeOfDayDomain); - } - - @Override - public int hashCode() { - return Objects.hash(super.hashCode(), name, type, project, labels); - } -} diff --git a/core/src/main/java/feast/core/model/Job.java b/core/src/main/java/feast/core/model/Job.java index fc801f76a44..e2278b5b17f 100644 --- a/core/src/main/java/feast/core/model/Job.java +++ b/core/src/main/java/feast/core/model/Job.java @@ -22,7 +22,6 @@ import feast.core.job.Runner; import java.util.ArrayList; import java.util.List; -import javax.persistence.CascadeType; import javax.persistence.Column; import javax.persistence.Entity; import javax.persistence.EnumType; @@ -33,7 +32,6 @@ import javax.persistence.JoinTable; import javax.persistence.ManyToMany; import javax.persistence.ManyToOne; -import javax.persistence.OneToMany; import javax.persistence.Table; import lombok.AllArgsConstructor; import lombok.Getter; @@ -82,10 +80,6 @@ public class Job extends AbstractTimestampEntity { }) private List featureSets; - // Job Metrics - @OneToMany(mappedBy = "job", cascade = CascadeType.ALL) - private List metrics; - @Enumerated(EnumType.STRING) @Column(name = "status", length = 16) private JobStatus status; @@ -119,11 +113,6 @@ public boolean isRunning() { return getStatus() == JobStatus.RUNNING; } - public void updateMetrics(List newMetrics) { - metrics.clear(); - metrics.addAll(newMetrics); - } - public String getSinkName() { return store.getName(); } diff --git a/core/src/main/java/feast/core/model/Metrics.java b/core/src/main/java/feast/core/model/Metrics.java deleted file mode 100644 index 0b7514816fa..00000000000 --- a/core/src/main/java/feast/core/model/Metrics.java +++ /dev/null @@ -1,64 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * Copyright 2018-2019 The Feast Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package feast.core.model; - -import javax.persistence.Entity; -import javax.persistence.FetchType; -import javax.persistence.GeneratedValue; -import javax.persistence.GenerationType; -import javax.persistence.Id; -import javax.persistence.JoinColumn; -import javax.persistence.ManyToOne; -import javax.persistence.Table; -import lombok.Getter; -import lombok.NoArgsConstructor; -import lombok.Setter; - -@NoArgsConstructor -@Getter -@Setter -@Entity -@Table(name = "metrics") -public class Metrics extends AbstractTimestampEntity { - - @Id - @GeneratedValue(strategy = GenerationType.AUTO) - private long id; - - @ManyToOne(fetch = FetchType.LAZY) - @JoinColumn(name = "job_id") - private Job job; - - /** Metrics name */ - private String name; - - /** Metrics value */ - private double value; - - /** - * Create a metrics owned by a {@code job}. - * - * @param job owner of this metrics. - * @param metricsName metrics name. - * @param value metrics value. - */ - public Metrics(Job job, String metricsName, double value) { - this.job = job; - this.name = metricsName; - this.value = value; - } -} diff --git a/core/src/test/java/feast/core/service/JobServiceTest.java b/core/src/test/java/feast/core/service/JobServiceTest.java index b649181afbf..1c6c349410c 100644 --- a/core/src/test/java/feast/core/service/JobServiceTest.java +++ b/core/src/test/java/feast/core/service/JobServiceTest.java @@ -34,6 +34,9 @@ import feast.core.CoreServiceProto.RestartIngestionJobResponse; import feast.core.CoreServiceProto.StopIngestionJobRequest; import feast.core.CoreServiceProto.StopIngestionJobResponse; +import feast.core.FeatureSetProto.EntitySpec; +import feast.core.FeatureSetProto.FeatureSetStatus; +import feast.core.FeatureSetProto.FeatureSpec; import feast.core.FeatureSetReferenceProto.FeatureSetReference; import feast.core.IngestionJobProto.IngestionJob; import feast.core.StoreProto.Store.RedisConfig; @@ -41,8 +44,9 @@ import feast.core.dao.JobRepository; import feast.core.job.JobManager; import feast.core.job.Runner; +import feast.core.model.Entity; +import feast.core.model.Feature; import feast.core.model.FeatureSet; -import feast.core.model.Field; import feast.core.model.Job; import feast.core.model.JobStatus; import feast.core.model.Source; diff --git a/core/src/test/java/feast/core/service/SpecServiceTest.java b/core/src/test/java/feast/core/service/SpecServiceTest.java index bb9f832bd7f..be9e7441b79 100644 --- a/core/src/test/java/feast/core/service/SpecServiceTest.java +++ b/core/src/test/java/feast/core/service/SpecServiceTest.java @@ -48,11 +48,8 @@ import feast.core.dao.ProjectRepository; import feast.core.dao.StoreRepository; import feast.core.exception.RetrievalException; -import feast.core.model.FeatureSet; -import feast.core.model.Field; -import feast.core.model.Project; -import feast.core.model.Source; -import feast.core.model.Store; +import feast.core.model.*; +import feast.types.ValueProto.ValueType; import feast.types.ValueProto.ValueType.Enum; import java.sql.Date; import java.time.Instant; diff --git a/protos/feast/core/FeatureSet.proto b/protos/feast/core/FeatureSet.proto index 9b60270a87a..f45d47d055d 100644 --- a/protos/feast/core/FeatureSet.proto +++ b/protos/feast/core/FeatureSet.proto @@ -71,46 +71,6 @@ message EntitySpec { // Value type of the feature. feast.types.ValueType.Enum value_type = 2; - - // presence_constraints, shape_type and domain_info are referenced from: - // https://github.com/tensorflow/metadata/blob/36f65d1268cbc92cdbcf812ee03dcf47fb53b91e/tensorflow_metadata/proto/v0/schema.proto#L107 - - oneof presence_constraints { - // Constraints on the presence of this feature in the examples. - tensorflow.metadata.v0.FeaturePresence presence = 3; - // Only used in the context of a "group" context, e.g., inside a sequence. - tensorflow.metadata.v0.FeaturePresenceWithinGroup group_presence = 4; - } - - // The shape of the feature which governs the number of values that appear in - // each example. - oneof shape_type { - // The feature has a fixed shape corresponding to a multi-dimensional - // tensor. - tensorflow.metadata.v0.FixedShape shape = 5; - // The feature doesn't have a well defined shape. All we know are limits on - // the minimum and maximum number of values. - tensorflow.metadata.v0.ValueCount value_count = 6; - } - - // Domain for the values of the feature. - oneof domain_info { - // Reference to a domain defined at the schema level. - string domain = 7; - // Inline definitions of domains. - tensorflow.metadata.v0.IntDomain int_domain = 8; - tensorflow.metadata.v0.FloatDomain float_domain = 9; - tensorflow.metadata.v0.StringDomain string_domain = 10; - tensorflow.metadata.v0.BoolDomain bool_domain = 11; - tensorflow.metadata.v0.StructDomain struct_domain = 12; - // Supported semantic domains. - tensorflow.metadata.v0.NaturalLanguageDomain natural_language_domain = 13; - tensorflow.metadata.v0.ImageDomain image_domain = 14; - tensorflow.metadata.v0.MIDDomain mid_domain = 15; - tensorflow.metadata.v0.URLDomain url_domain = 16; - tensorflow.metadata.v0.TimeDomain time_domain = 17; - tensorflow.metadata.v0.TimeOfDayDomain time_of_day_domain = 18; - } } message FeatureSpec { diff --git a/sdk/python/feast/entity.py b/sdk/python/feast/entity.py index 9c5a027b974..012d01631af 100644 --- a/sdk/python/feast/entity.py +++ b/sdk/python/feast/entity.py @@ -29,26 +29,7 @@ def to_proto(self) -> EntityProto: Returns EntitySpec object """ value_type = ValueTypeProto.ValueType.Enum.Value(self.dtype.name) - return EntityProto( - name=self.name, - value_type=value_type, - presence=self.presence, - group_presence=self.group_presence, - shape=self.shape, - value_count=self.value_count, - domain=self.domain, - int_domain=self.int_domain, - float_domain=self.float_domain, - string_domain=self.string_domain, - bool_domain=self.bool_domain, - struct_domain=self.struct_domain, - natural_language_domain=self.natural_language_domain, - image_domain=self.image_domain, - mid_domain=self.mid_domain, - url_domain=self.url_domain, - time_domain=self.time_domain, - time_of_day_domain=self.time_of_day_domain, - ) + return EntityProto(name=self.name, value_type=value_type,) @classmethod def from_proto(cls, entity_proto: EntityProto): @@ -62,7 +43,4 @@ def from_proto(cls, entity_proto: EntityProto): Entity object """ entity = cls(name=entity_proto.name, dtype=ValueType(entity_proto.value_type)) - entity.update_presence_constraints(entity_proto) - entity.update_shape_type(entity_proto) - entity.update_domain_info(entity_proto) return entity diff --git a/sdk/python/feast/feature_set.py b/sdk/python/feast/feature_set.py index 760e947318f..ace7f165de1 100644 --- a/sdk/python/feast/feature_set.py +++ b/sdk/python/feast/feature_set.py @@ -716,6 +716,8 @@ def export_tfx_schema(self) -> schema_pb2.Schema: ] for _, field in self._fields.items(): + if isinstance(field, Entity): + continue feature = schema_pb2.Feature() for attr in attributes_to_copy_from_field_to_feature: if getattr(field, attr) is None: diff --git a/sdk/python/tests/test_feature_set.py b/sdk/python/tests/test_feature_set.py index 0a7d1ebabea..a2cc12fe113 100644 --- a/sdk/python/tests/test_feature_set.py +++ b/sdk/python/tests/test_feature_set.py @@ -210,9 +210,6 @@ def test_import_tfx_schema(self): feature_set.import_tfx_schema(test_input_schema) # After update - for entity in feature_set.entities: - assert entity.presence is not None - assert entity.shape is not None for feature in feature_set.features: assert feature.presence is not None assert feature.shape is not None From 17904dab1a3761c3e84f03e8e64bc1f036599975 Mon Sep 17 00:00:00 2001 From: zhilingc Date: Mon, 27 Apr 2020 09:13:08 +0800 Subject: [PATCH 02/11] Remove TFX fields for entities in testdata --- .../bikeshare_feature_set.yaml | 9 --------- .../tensorflow_metadata/bikeshare_schema.json | 19 ------------------- 2 files changed, 28 deletions(-) diff --git a/sdk/python/tests/data/tensorflow_metadata/bikeshare_feature_set.yaml b/sdk/python/tests/data/tensorflow_metadata/bikeshare_feature_set.yaml index daa0a35f0ab..48c595712cb 100644 --- a/sdk/python/tests/data/tensorflow_metadata/bikeshare_feature_set.yaml +++ b/sdk/python/tests/data/tensorflow_metadata/bikeshare_feature_set.yaml @@ -3,15 +3,6 @@ spec: entities: - name: station_id valueType: INT64 - intDomain: - min: 1 - max: 5000 - presence: - minFraction: 1.0 - minCount: 1 - shape: - dim: - - size: 1 features: - name: location valueType: STRING diff --git a/sdk/python/tests/data/tensorflow_metadata/bikeshare_schema.json b/sdk/python/tests/data/tensorflow_metadata/bikeshare_schema.json index e7a886053c1..fa9f97cca0d 100644 --- a/sdk/python/tests/data/tensorflow_metadata/bikeshare_schema.json +++ b/sdk/python/tests/data/tensorflow_metadata/bikeshare_schema.json @@ -85,25 +85,6 @@ } ] } - }, - { - "name": "station_id", - "type": "INT", - "presence": { - "minFraction": 1.0, - "minCount": "1" - }, - "int_domain": { - "min": 1, - "max": 5000 - }, - "shape": { - "dim": [ - { - "size": "1" - } - ] - } } ], "stringDomain": [ From 9bcbabed076856305596f4de9e66c5b946f7e0bc Mon Sep 17 00:00:00 2001 From: zhilingc Date: Wed, 29 Apr 2020 20:59:39 +0800 Subject: [PATCH 03/11] Split Field model into distinct Feature and Entity objects --- core/src/main/java/feast/core/model/Feature.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/feast/core/model/Feature.java b/core/src/main/java/feast/core/model/Feature.java index 907e3446a1b..f2809481cd2 100644 --- a/core/src/main/java/feast/core/model/Feature.java +++ b/core/src/main/java/feast/core/model/Feature.java @@ -64,7 +64,7 @@ public class Feature { private byte[] timeDomain; private byte[] timeOfDayDomain; - private Feature() {} + public Feature() {} private Feature(String name, ValueType.Enum type) { this.setReference(new FeatureReference(name)); From 2c0a86fc593d17dd31fe8c7407c4653f21122b6f Mon Sep 17 00:00:00 2001 From: zhilingc Date: Thu, 30 Apr 2020 11:38:45 +0800 Subject: [PATCH 04/11] Index jointables --- .../java/feast/core/model/FeatureSet.java | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/core/src/main/java/feast/core/model/FeatureSet.java b/core/src/main/java/feast/core/model/FeatureSet.java index 71cae52737d..b80df5ea04f 100644 --- a/core/src/main/java/feast/core/model/FeatureSet.java +++ b/core/src/main/java/feast/core/model/FeatureSet.java @@ -59,10 +59,38 @@ public class FeatureSet extends AbstractTimestampEntity implements Comparable entities; // Feature fields inside this feature set @OneToMany(cascade = CascadeType.ALL, fetch = FetchType.EAGER) + @JoinTable( + name = "feature_set_features", + joinColumns = @JoinColumn(name = "feature_set_id"), + inverseJoinColumns = { + @JoinColumn(name = "features_name"), + @JoinColumn(name = "features_project"), + @JoinColumn(name = "features_feature_set_id"), + @JoinColumn(name = "features_version") + }, + indexes = { + @Index( + name = "idx_jobs_feature_set_features_feature_set_id", + columnList = "feature_set_id"), + }) private Set features; // Source on which feature rows can be found From 4939b6caa400bdfb6951d6d9f851cffd1ad817b2 Mon Sep 17 00:00:00 2001 From: zhilingc Date: Thu, 30 Apr 2020 17:15:37 +0800 Subject: [PATCH 05/11] Explicitly name tables, remove redundant constructor --- core/src/main/java/feast/core/model/Entity.java | 2 ++ .../src/main/java/feast/core/model/Feature.java | 1 + core/src/main/java/feast/core/model/Job.java | 17 ----------------- .../java/feast/core/service/JobServiceTest.java | 5 ----- .../feast/core/service/SpecServiceTest.java | 1 - 5 files changed, 3 insertions(+), 23 deletions(-) diff --git a/core/src/main/java/feast/core/model/Entity.java b/core/src/main/java/feast/core/model/Entity.java index b9d2a8827f2..04b3fa9c857 100644 --- a/core/src/main/java/feast/core/model/Entity.java +++ b/core/src/main/java/feast/core/model/Entity.java @@ -20,6 +20,7 @@ import feast.types.ValueProto.ValueType; import java.util.Objects; import javax.persistence.EmbeddedId; +import javax.persistence.Table; import lombok.Getter; import lombok.Setter; @@ -27,6 +28,7 @@ @Getter @Setter @javax.persistence.Entity +@Table(name = "entities") public class Entity { @EmbeddedId private EntityReference reference; diff --git a/core/src/main/java/feast/core/model/Feature.java b/core/src/main/java/feast/core/model/Feature.java index f2809481cd2..277f3c81d5b 100644 --- a/core/src/main/java/feast/core/model/Feature.java +++ b/core/src/main/java/feast/core/model/Feature.java @@ -32,6 +32,7 @@ @Getter @Setter @Entity +@Table(name = "features") public class Feature { @EmbeddedId private FeatureReference reference; diff --git a/core/src/main/java/feast/core/model/Job.java b/core/src/main/java/feast/core/model/Job.java index e2278b5b17f..b31b9b7e61a 100644 --- a/core/src/main/java/feast/core/model/Job.java +++ b/core/src/main/java/feast/core/model/Job.java @@ -88,23 +88,6 @@ public Job() { super(); } - public Job( - String id, - String extId, - Runner runner, - Source source, - Store sink, - List featureSets, - JobStatus jobStatus) { - this.id = id; - this.extId = extId; - this.source = source; - this.runner = runner; - this.store = sink; - this.featureSets = featureSets; - this.status = jobStatus; - } - public boolean hasTerminated() { return getStatus().isTerminal(); } diff --git a/core/src/test/java/feast/core/service/JobServiceTest.java b/core/src/test/java/feast/core/service/JobServiceTest.java index 1c6c349410c..42128a16d64 100644 --- a/core/src/test/java/feast/core/service/JobServiceTest.java +++ b/core/src/test/java/feast/core/service/JobServiceTest.java @@ -34,9 +34,6 @@ import feast.core.CoreServiceProto.RestartIngestionJobResponse; import feast.core.CoreServiceProto.StopIngestionJobRequest; import feast.core.CoreServiceProto.StopIngestionJobResponse; -import feast.core.FeatureSetProto.EntitySpec; -import feast.core.FeatureSetProto.FeatureSetStatus; -import feast.core.FeatureSetProto.FeatureSpec; import feast.core.FeatureSetReferenceProto.FeatureSetReference; import feast.core.IngestionJobProto.IngestionJob; import feast.core.StoreProto.Store.RedisConfig; @@ -44,8 +41,6 @@ import feast.core.dao.JobRepository; import feast.core.job.JobManager; import feast.core.job.Runner; -import feast.core.model.Entity; -import feast.core.model.Feature; import feast.core.model.FeatureSet; import feast.core.model.Job; import feast.core.model.JobStatus; diff --git a/core/src/test/java/feast/core/service/SpecServiceTest.java b/core/src/test/java/feast/core/service/SpecServiceTest.java index be9e7441b79..0341185f054 100644 --- a/core/src/test/java/feast/core/service/SpecServiceTest.java +++ b/core/src/test/java/feast/core/service/SpecServiceTest.java @@ -49,7 +49,6 @@ import feast.core.dao.StoreRepository; import feast.core.exception.RetrievalException; import feast.core.model.*; -import feast.types.ValueProto.ValueType; import feast.types.ValueProto.ValueType.Enum; import java.sql.Date; import java.time.Instant; From 25b8da962a46b0f04872c473542f6fb6aa45eb80 Mon Sep 17 00:00:00 2001 From: zhilingc Date: Thu, 30 Apr 2020 17:53:49 +0800 Subject: [PATCH 06/11] Integrate labels --- .../main/java/feast/core/model/Feature.java | 14 +++- .../java/feast/core/model/FeatureSet.java | 1 + .../feast/core/service/JobServiceTest.java | 10 +-- .../feast/core/service/SpecServiceTest.java | 73 +++++-------------- .../feast/core/service/TestObjectFactory.java | 13 ++-- 5 files changed, 43 insertions(+), 68 deletions(-) diff --git a/core/src/main/java/feast/core/model/Feature.java b/core/src/main/java/feast/core/model/Feature.java index 277f3c81d5b..eb29c3b923d 100644 --- a/core/src/main/java/feast/core/model/Feature.java +++ b/core/src/main/java/feast/core/model/Feature.java @@ -17,8 +17,10 @@ package feast.core.model; import feast.core.FeatureSetProto.FeatureSpec; +import feast.core.util.TypeConversion; import feast.types.ValueProto.ValueType; import java.util.Arrays; +import java.util.Map; import java.util.Objects; import javax.persistence.*; import javax.persistence.Entity; @@ -40,6 +42,10 @@ public class Feature { // Type of the field private String type; + // Labels that this field belongs to + @Column(name = "labels", columnDefinition = "text") + private String labels; + // Presence constraints (refer to proto feast.core.FeatureSet.FeatureSpec) // Only one of them can be set. private byte[] presence; @@ -80,6 +86,7 @@ public static Feature withReference(FeatureReference featureRef) { public static Feature fromProto(FeatureSpec featureSpec) { Feature feature = new Feature(featureSpec.getName(), featureSpec.getValueType()); + feature.labels = TypeConversion.convertMapToJsonString(featureSpec.getLabelsMap()); switch (featureSpec.getPresenceConstraintsCase()) { case PRESENCE: @@ -146,6 +153,10 @@ public static Feature fromProto(FeatureSpec featureSpec) { return feature; } + public Map getLabels() { + return TypeConversion.convertJsonStringToMap(this.labels); + } + @Override public boolean equals(Object o) { if (this == o) { @@ -156,6 +167,7 @@ public boolean equals(Object o) { } Feature feature = (Feature) o; return Objects.equals(getReference(), feature.getReference()) + && Objects.equals(labels, feature.labels) && Arrays.equals(getPresence(), feature.getPresence()) && Arrays.equals(getGroupPresence(), feature.getGroupPresence()) && Arrays.equals(getShape(), feature.getShape()) @@ -176,6 +188,6 @@ public boolean equals(Object o) { @Override public int hashCode() { - return Objects.hash(super.hashCode(), getReference(), getType()); + return Objects.hash(super.hashCode(), getReference(), getType(), labels); } } diff --git a/core/src/main/java/feast/core/model/FeatureSet.java b/core/src/main/java/feast/core/model/FeatureSet.java index b80df5ea04f..64198cfa075 100644 --- a/core/src/main/java/feast/core/model/FeatureSet.java +++ b/core/src/main/java/feast/core/model/FeatureSet.java @@ -21,6 +21,7 @@ import com.google.protobuf.Timestamp; import feast.core.FeatureSetProto; import feast.core.FeatureSetProto.*; +import feast.core.util.TypeConversion; import feast.types.ValueProto.ValueType.Enum; import java.util.*; import javax.persistence.*; diff --git a/core/src/test/java/feast/core/service/JobServiceTest.java b/core/src/test/java/feast/core/service/JobServiceTest.java index 42128a16d64..ba663020191 100644 --- a/core/src/test/java/feast/core/service/JobServiceTest.java +++ b/core/src/test/java/feast/core/service/JobServiceTest.java @@ -41,11 +41,7 @@ import feast.core.dao.JobRepository; import feast.core.job.JobManager; import feast.core.job.Runner; -import feast.core.model.FeatureSet; -import feast.core.model.Job; -import feast.core.model.JobStatus; -import feast.core.model.Source; -import feast.core.model.Store; +import feast.core.model.*; import feast.types.ValueProto.ValueType.Enum; import java.time.Instant; import java.util.ArrayList; @@ -147,8 +143,8 @@ public void setupJobManager() { // dummy model constructorss private FeatureSet newDummyFeatureSet(String name, int version, String project) { - Field feature = TestObjectFactory.CreateFeatureField(name + "_feature", Enum.INT64); - Field entity = TestObjectFactory.CreateEntityField(name + "_entity", Enum.STRING); + Feature feature = TestObjectFactory.CreateFeature(name + "_feature", Enum.INT64); + Entity entity = TestObjectFactory.CreateEntity(name + "_entity", Enum.STRING); FeatureSet fs = TestObjectFactory.CreateFeatureSet( diff --git a/core/src/test/java/feast/core/service/SpecServiceTest.java b/core/src/test/java/feast/core/service/SpecServiceTest.java index 0341185f054..413a97e64b0 100644 --- a/core/src/test/java/feast/core/service/SpecServiceTest.java +++ b/core/src/test/java/feast/core/service/SpecServiceTest.java @@ -110,9 +110,9 @@ public void setUp() { FeatureSet featureSet1v3 = newDummyFeatureSet("f1", 3, "project1"); FeatureSet featureSet2v1 = newDummyFeatureSet("f2", 1, "project1"); - Field f3f1 = TestObjectFactory.CreateFeatureField("f3f1", Enum.INT64); - Field f3f2 = TestObjectFactory.CreateFeatureField("f3f2", Enum.INT64); - Field f3e1 = TestObjectFactory.CreateEntityField("f3e1", Enum.STRING); + Feature f3f1 = TestObjectFactory.CreateFeature("f3f1", Enum.INT64); + Feature f3f2 = TestObjectFactory.CreateFeature("f3f2", Enum.INT64); + Entity f3e1 = TestObjectFactory.CreateEntity("f3e1", Enum.STRING); FeatureSet featureSet3v1 = TestObjectFactory.CreateFeatureSet( "f3", "project1", 1, Arrays.asList(f3e1), Arrays.asList(f3f2, f3f1)); @@ -468,9 +468,9 @@ public void applyFeatureSetShouldIncrementFeatureSetVersionIfAlreadyExists() public void applyFeatureSetShouldNotCreateFeatureSetIfFieldsUnordered() throws InvalidProtocolBufferException { - Field f3f1 = TestObjectFactory.CreateFeatureField("f3f1", Enum.INT64); - Field f3f2 = TestObjectFactory.CreateFeatureField("f3f2", Enum.INT64); - Field f3e1 = TestObjectFactory.CreateEntityField("f3e1", Enum.STRING); + Feature f3f1 = TestObjectFactory.CreateFeature("f3f1", Enum.INT64); + Feature f3f2 = TestObjectFactory.CreateFeature("f3f2", Enum.INT64); + Entity f3e1 = TestObjectFactory.CreateEntity("f3e1", Enum.STRING); FeatureSetProto.FeatureSet incomingFeatureSet = (TestObjectFactory.CreateFeatureSet( "f3", "project1", 5, Arrays.asList(f3e1), Arrays.asList(f3f2, f3f1))) @@ -494,46 +494,11 @@ public void applyFeatureSetShouldNotCreateFeatureSetIfFieldsUnordered() public void applyFeatureSetShouldAcceptPresenceShapeAndDomainConstraints() throws InvalidProtocolBufferException { List entitySpecs = new ArrayList<>(); - entitySpecs.add( - EntitySpec.newBuilder() - .setName("entity1") - .setValueType(Enum.INT64) - .setPresence(FeaturePresence.getDefaultInstance()) - .setShape(FixedShape.getDefaultInstance()) - .setDomain("mydomain") - .build()); - entitySpecs.add( - EntitySpec.newBuilder() - .setName("entity2") - .setValueType(Enum.INT64) - .setGroupPresence(FeaturePresenceWithinGroup.getDefaultInstance()) - .setValueCount(ValueCount.getDefaultInstance()) - .setIntDomain(IntDomain.getDefaultInstance()) - .build()); - entitySpecs.add( - EntitySpec.newBuilder() - .setName("entity3") - .setValueType(Enum.FLOAT) - .setPresence(FeaturePresence.getDefaultInstance()) - .setValueCount(ValueCount.getDefaultInstance()) - .setFloatDomain(FloatDomain.getDefaultInstance()) - .build()); - entitySpecs.add( - EntitySpec.newBuilder() - .setName("entity4") - .setValueType(Enum.STRING) - .setPresence(FeaturePresence.getDefaultInstance()) - .setValueCount(ValueCount.getDefaultInstance()) - .setStringDomain(StringDomain.getDefaultInstance()) - .build()); - entitySpecs.add( - EntitySpec.newBuilder() - .setName("entity5") - .setValueType(Enum.BOOL) - .setPresence(FeaturePresence.getDefaultInstance()) - .setValueCount(ValueCount.getDefaultInstance()) - .setBoolDomain(BoolDomain.getDefaultInstance()) - .build()); + entitySpecs.add(EntitySpec.newBuilder().setName("entity1").setValueType(Enum.INT64).build()); + entitySpecs.add(EntitySpec.newBuilder().setName("entity2").setValueType(Enum.INT64).build()); + entitySpecs.add(EntitySpec.newBuilder().setName("entity3").setValueType(Enum.FLOAT).build()); + entitySpecs.add(EntitySpec.newBuilder().setName("entity4").setValueType(Enum.STRING).build()); + entitySpecs.add(EntitySpec.newBuilder().setName("entity5").setValueType(Enum.BOOL).build()); List featureSpecs = new ArrayList<>(); featureSpecs.add( @@ -676,9 +641,9 @@ public void applyFeatureSetShouldUpdateFeatureSetWhenConstraintsAreUpdated() @Test public void applyFeatureSetShouldCreateProjectWhenNotAlreadyExists() throws InvalidProtocolBufferException { - Field f3f1 = TestObjectFactory.CreateFeatureField("f3f1", Enum.INT64); - Field f3f2 = TestObjectFactory.CreateFeatureField("f3f2", Enum.INT64); - Field f3e1 = TestObjectFactory.CreateEntityField("f3e1", Enum.STRING); + Feature f3f1 = TestObjectFactory.CreateFeature("f3f1", Enum.INT64); + Feature f3f2 = TestObjectFactory.CreateFeature("f3f2", Enum.INT64); + Entity f3e1 = TestObjectFactory.CreateEntity("f3e1", Enum.STRING); FeatureSetProto.FeatureSet incomingFeatureSet = (TestObjectFactory.CreateFeatureSet( "f3", "newproject", 5, Arrays.asList(f3e1), Arrays.asList(f3f2, f3f1))) @@ -695,9 +660,9 @@ public void applyFeatureSetShouldCreateProjectWhenNotAlreadyExists() @Test public void applyFeatureSetShouldFailWhenProjectIsArchived() throws InvalidProtocolBufferException { - Field f3f1 = TestObjectFactory.CreateFeatureField("f3f1", Enum.INT64); - Field f3f2 = TestObjectFactory.CreateFeatureField("f3f2", Enum.INT64); - Field f3e1 = TestObjectFactory.CreateEntityField("f3e1", Enum.STRING); + Feature f3f1 = TestObjectFactory.CreateFeature("f3f1", Enum.INT64); + Feature f3f2 = TestObjectFactory.CreateFeature("f3f2", Enum.INT64); + Entity f3e1 = TestObjectFactory.CreateEntity("f3e1", Enum.STRING); FeatureSetProto.FeatureSet incomingFeatureSet = (TestObjectFactory.CreateFeatureSet( "f3", "archivedproject", 5, Arrays.asList(f3e1), Arrays.asList(f3f2, f3f1))) @@ -856,8 +821,8 @@ private FeatureSet newDummyFeatureSet(String name, int version, String project) .setValueType(Enum.STRING) .putLabels("key", "value") .build(); - Field feature = new Field(f1); - Field entity = TestObjectFactory.CreateEntityField("entity", Enum.STRING); + Feature feature = Feature.fromProto(f1); + Entity entity = TestObjectFactory.CreateEntity("entity", Enum.STRING); FeatureSet fs = TestObjectFactory.CreateFeatureSet( diff --git a/core/src/test/java/feast/core/service/TestObjectFactory.java b/core/src/test/java/feast/core/service/TestObjectFactory.java index 966cb8d8163..0476dbe5c2e 100644 --- a/core/src/test/java/feast/core/service/TestObjectFactory.java +++ b/core/src/test/java/feast/core/service/TestObjectFactory.java @@ -18,8 +18,9 @@ import feast.core.FeatureSetProto; import feast.core.SourceProto; +import feast.core.model.Entity; +import feast.core.model.Feature; import feast.core.model.FeatureSet; -import feast.core.model.Field; import feast.core.model.Source; import feast.types.ValueProto; import java.util.HashMap; @@ -37,7 +38,7 @@ public class TestObjectFactory { true); public static FeatureSet CreateFeatureSet( - String name, String project, int version, List entities, List features) { + String name, String project, int version, List entities, List features) { return new FeatureSet( name, project, @@ -50,13 +51,13 @@ public static FeatureSet CreateFeatureSet( FeatureSetProto.FeatureSetStatus.STATUS_READY); } - public static Field CreateFeatureField(String name, ValueProto.ValueType.Enum valueType) { - return new Field( + public static Feature CreateFeature(String name, ValueProto.ValueType.Enum valueType) { + return Feature.fromProto( FeatureSetProto.FeatureSpec.newBuilder().setName(name).setValueType(valueType).build()); } - public static Field CreateEntityField(String name, ValueProto.ValueType.Enum valueType) { - return new Field( + public static Entity CreateEntity(String name, ValueProto.ValueType.Enum valueType) { + return Entity.fromProto( FeatureSetProto.EntitySpec.newBuilder().setName(name).setValueType(valueType).build()); } } From fc6c498b6be2c87713221b660b0f45b928524eab Mon Sep 17 00:00:00 2001 From: zhilingc Date: Fri, 1 May 2020 09:53:48 +0800 Subject: [PATCH 07/11] Fix code comments --- core/src/main/java/feast/core/model/Entity.java | 2 +- core/src/main/java/feast/core/model/EntityReference.java | 8 ++++---- core/src/main/java/feast/core/model/Feature.java | 6 +++--- core/src/main/java/feast/core/model/FeatureReference.java | 8 ++++---- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/core/src/main/java/feast/core/model/Entity.java b/core/src/main/java/feast/core/model/Entity.java index 04b3fa9c857..24bafdd4034 100644 --- a/core/src/main/java/feast/core/model/Entity.java +++ b/core/src/main/java/feast/core/model/Entity.java @@ -24,7 +24,7 @@ import lombok.Getter; import lombok.Setter; -/** Feast entity object. Contains name, type as well as domain metadata about the entity. */ +/** Feast entity object. Contains name and type of the entity. */ @Getter @Setter @javax.persistence.Entity diff --git a/core/src/main/java/feast/core/model/EntityReference.java b/core/src/main/java/feast/core/model/EntityReference.java index 88d808df7fc..fb05c067c63 100644 --- a/core/src/main/java/feast/core/model/EntityReference.java +++ b/core/src/main/java/feast/core/model/EntityReference.java @@ -31,19 +31,19 @@ @Getter @Setter public class EntityReference implements Serializable { - // Project the field belongs to + // Project the entity belongs to @Column(nullable = false) private String project; - // Feature set the field belongs to + // Feature set the entity belongs to @Column(name = "feature_set", nullable = false) private String featureSet; - // Version of the feature set this field belongs to + // Version of the feature set this entity belongs to @Column(nullable = false) private int version; - // Name of the field + // Name of the entity @Column(nullable = false) private String name; diff --git a/core/src/main/java/feast/core/model/Feature.java b/core/src/main/java/feast/core/model/Feature.java index eb29c3b923d..03e91d3dd17 100644 --- a/core/src/main/java/feast/core/model/Feature.java +++ b/core/src/main/java/feast/core/model/Feature.java @@ -39,10 +39,10 @@ public class Feature { @EmbeddedId private FeatureReference reference; - // Type of the field + /** Data type of the feature. String representation of {@link ValueType} * */ private String type; - // Labels that this field belongs to + // Labels for this feature @Column(name = "labels", columnDefinition = "text") private String labels; @@ -78,7 +78,7 @@ private Feature(String name, ValueType.Enum type) { this.setType(type.toString()); } - public static Feature withReference(FeatureReference featureRef) { + public static Feature withRef(FeatureReference featureRef) { Feature feature = new Feature(); feature.setReference(featureRef); return feature; diff --git a/core/src/main/java/feast/core/model/FeatureReference.java b/core/src/main/java/feast/core/model/FeatureReference.java index b27c17f1b94..bee7a17955d 100644 --- a/core/src/main/java/feast/core/model/FeatureReference.java +++ b/core/src/main/java/feast/core/model/FeatureReference.java @@ -31,19 +31,19 @@ @Getter @Setter public class FeatureReference implements Serializable { - // Project the field belongs to + // Project the feature belongs to @Column(nullable = false) private String project; - // Feature set the field belongs to + // Feature set the feature belongs to @Column(name = "feature_set", nullable = false) private String featureSet; - // Version of the feature set this field belongs to + // Version of the feature set this feature belongs to @Column(nullable = false) private int version; - // Name of the field + // Name of the feature @Column(nullable = false) private String name; From 7031753bdfbce8a0bfc63c1477f537ddbf57492f Mon Sep 17 00:00:00 2001 From: zhilingc Date: Fri, 1 May 2020 14:37:37 +0800 Subject: [PATCH 08/11] Change FeatureSetId to int --- core/src/main/java/feast/core/job/JobUpdateTask.java | 8 ++++++-- core/src/main/java/feast/core/model/FeatureSet.java | 12 ++---------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/core/src/main/java/feast/core/job/JobUpdateTask.java b/core/src/main/java/feast/core/job/JobUpdateTask.java index 25ce386d40c..985e4eb31e6 100644 --- a/core/src/main/java/feast/core/job/JobUpdateTask.java +++ b/core/src/main/java/feast/core/job/JobUpdateTask.java @@ -103,9 +103,13 @@ public Job call() { boolean featureSetsChangedFor(Job job) { Set existingFeatureSetsPopulatedByJob = - job.getFeatureSets().stream().map(FeatureSet::getId).collect(Collectors.toSet()); + job.getFeatureSets().stream() + .map(fs -> fs.getProject() + "/" + fs.getName()) + .collect(Collectors.toSet()); Set newFeatureSetsPopulatedByJob = - featureSets.stream().map(FeatureSet::getId).collect(Collectors.toSet()); + featureSets.stream() + .map(fs -> fs.getProject() + "/" + fs.getName()) + .collect(Collectors.toSet()); return !newFeatureSetsPopulatedByJob.equals(existingFeatureSetsPopulatedByJob); } diff --git a/core/src/main/java/feast/core/model/FeatureSet.java b/core/src/main/java/feast/core/model/FeatureSet.java index 64198cfa075..a6e46a2d747 100644 --- a/core/src/main/java/feast/core/model/FeatureSet.java +++ b/core/src/main/java/feast/core/model/FeatureSet.java @@ -38,8 +38,8 @@ public class FeatureSet extends AbstractTimestampEntity implements Comparable Date: Fri, 1 May 2020 16:01:03 +0800 Subject: [PATCH 09/11] Retrieve featuresets from repository so that ids are consistent --- core/src/main/java/feast/core/model/Job.java | 13 ++----------- .../feast/core/service/JobCoordinatorService.java | 11 ++++++++--- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/core/src/main/java/feast/core/model/Job.java b/core/src/main/java/feast/core/model/Job.java index b31b9b7e61a..5c812f2a9ca 100644 --- a/core/src/main/java/feast/core/model/Job.java +++ b/core/src/main/java/feast/core/model/Job.java @@ -22,17 +22,8 @@ import feast.core.job.Runner; import java.util.ArrayList; import java.util.List; -import javax.persistence.Column; +import javax.persistence.*; import javax.persistence.Entity; -import javax.persistence.EnumType; -import javax.persistence.Enumerated; -import javax.persistence.Id; -import javax.persistence.Index; -import javax.persistence.JoinColumn; -import javax.persistence.JoinTable; -import javax.persistence.ManyToMany; -import javax.persistence.ManyToOne; -import javax.persistence.Table; import lombok.AllArgsConstructor; import lombok.Getter; import lombok.Setter; @@ -69,7 +60,7 @@ public class Job extends AbstractTimestampEntity { private Store store; // FeatureSets populated by the job - @ManyToMany + @ManyToMany(cascade = CascadeType.ALL) @JoinTable( name = "jobs_feature_sets", joinColumns = @JoinColumn(name = "job_id"), diff --git a/core/src/main/java/feast/core/service/JobCoordinatorService.java b/core/src/main/java/feast/core/service/JobCoordinatorService.java index 6f366be5083..a0d77ca232e 100644 --- a/core/src/main/java/feast/core/service/JobCoordinatorService.java +++ b/core/src/main/java/feast/core/service/JobCoordinatorService.java @@ -195,8 +195,13 @@ public Optional getJob(Source source, Store store) { return Optional.of(jobs.get(0)); } - // TODO: Put in a util somewhere? - private static List featureSetsFromProto(List protos) { - return protos.stream().map(FeatureSet::fromProto).collect(Collectors.toList()); + private List featureSetsFromProto(List protos) { + return protos.stream() + .map(FeatureSetProto.FeatureSet::getSpec) + .map( + fs -> + featureSetRepository.findFeatureSetByNameAndProject_NameAndVersion( + fs.getName(), fs.getProject(), fs.getVersion())) + .collect(Collectors.toList()); } } From d579424b88388bdd33c6d2bd30cfd2e0e5a37b8d Mon Sep 17 00:00:00 2001 From: zhilingc Date: Fri, 1 May 2020 18:17:07 +0800 Subject: [PATCH 10/11] Add uniqueness constraint to FeatureSets, fix tests --- .../main/java/feast/core/job/JobUpdateTask.java | 12 +++--------- core/src/main/java/feast/core/model/Entity.java | 1 + .../src/main/java/feast/core/model/FeatureSet.java | 4 +++- .../feast/core/service/JobCoordinatorService.java | 1 + .../core/service/JobCoordinatorServiceTest.java | 14 ++++++++++++++ protos/feast/core/FeatureSet.proto | 2 +- 6 files changed, 23 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/feast/core/job/JobUpdateTask.java b/core/src/main/java/feast/core/job/JobUpdateTask.java index 985e4eb31e6..bb876f47f22 100644 --- a/core/src/main/java/feast/core/job/JobUpdateTask.java +++ b/core/src/main/java/feast/core/job/JobUpdateTask.java @@ -16,6 +16,7 @@ */ package feast.core.job; +import com.google.common.collect.Sets; import feast.core.log.Action; import feast.core.log.AuditLogger; import feast.core.log.Resource; @@ -35,7 +36,6 @@ import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; -import java.util.stream.Collectors; import lombok.Getter; import lombok.extern.slf4j.Slf4j; @@ -102,14 +102,8 @@ public Job call() { } boolean featureSetsChangedFor(Job job) { - Set existingFeatureSetsPopulatedByJob = - job.getFeatureSets().stream() - .map(fs -> fs.getProject() + "/" + fs.getName()) - .collect(Collectors.toSet()); - Set newFeatureSetsPopulatedByJob = - featureSets.stream() - .map(fs -> fs.getProject() + "/" + fs.getName()) - .collect(Collectors.toSet()); + Set existingFeatureSetsPopulatedByJob = Sets.newHashSet(job.getFeatureSets()); + Set newFeatureSetsPopulatedByJob = Sets.newHashSet(featureSets); return !newFeatureSetsPopulatedByJob.equals(existingFeatureSetsPopulatedByJob); } diff --git a/core/src/main/java/feast/core/model/Entity.java b/core/src/main/java/feast/core/model/Entity.java index 24bafdd4034..4148b8a2a15 100644 --- a/core/src/main/java/feast/core/model/Entity.java +++ b/core/src/main/java/feast/core/model/Entity.java @@ -32,6 +32,7 @@ public class Entity { @EmbeddedId private EntityReference reference; + /** Data type of the entity. String representation of {@link ValueType} * */ private String type; public Entity() {} diff --git a/core/src/main/java/feast/core/model/FeatureSet.java b/core/src/main/java/feast/core/model/FeatureSet.java index a6e46a2d747..8ffe334d355 100644 --- a/core/src/main/java/feast/core/model/FeatureSet.java +++ b/core/src/main/java/feast/core/model/FeatureSet.java @@ -33,7 +33,9 @@ @Getter @Setter @javax.persistence.Entity -@Table(name = "feature_sets") +@Table( + name = "feature_sets", + uniqueConstraints = @UniqueConstraint(columnNames = {"name", "version", "project_name"})) public class FeatureSet extends AbstractTimestampEntity implements Comparable { // Id of the featureSet, defined as project/feature_set_name:feature_set_version diff --git a/core/src/main/java/feast/core/service/JobCoordinatorService.java b/core/src/main/java/feast/core/service/JobCoordinatorService.java index a0d77ca232e..c0215767905 100644 --- a/core/src/main/java/feast/core/service/JobCoordinatorService.java +++ b/core/src/main/java/feast/core/service/JobCoordinatorService.java @@ -195,6 +195,7 @@ public Optional getJob(Source source, Store store) { return Optional.of(jobs.get(0)); } + // TODO: optimize this to make less calls to the database. private List featureSetsFromProto(List protos) { return protos.stream() .map(FeatureSetProto.FeatureSet::getSpec) diff --git a/core/src/test/java/feast/core/service/JobCoordinatorServiceTest.java b/core/src/test/java/feast/core/service/JobCoordinatorServiceTest.java index 59fdc32b20f..26cf331c13b 100644 --- a/core/src/test/java/feast/core/service/JobCoordinatorServiceTest.java +++ b/core/src/test/java/feast/core/service/JobCoordinatorServiceTest.java @@ -25,6 +25,7 @@ import static org.mockito.Mockito.when; import static org.mockito.MockitoAnnotations.initMocks; +import com.google.common.collect.Lists; import com.google.protobuf.InvalidProtocolBufferException; import feast.core.CoreServiceProto.ListFeatureSetsRequest.Filter; import feast.core.CoreServiceProto.ListFeatureSetsResponse; @@ -194,6 +195,13 @@ public void shouldGenerateAndSubmitJobsIfAny() throws InvalidProtocolBufferExcep when(specService.listStores(any())) .thenReturn(ListStoresResponse.newBuilder().addStore(store).build()); + for (FeatureSetProto.FeatureSet fs : Lists.newArrayList(featureSet1, featureSet2)) { + FeatureSetSpec spec = fs.getSpec(); + when(featureSetRepository.findFeatureSetByNameAndProject_NameAndVersion( + spec.getName(), spec.getProject(), spec.getVersion())) + .thenReturn(FeatureSet.fromProto(fs)); + } + when(jobManager.startJob(argThat(new JobMatcher(expectedInput)))).thenReturn(expected); when(jobManager.getRunnerType()).thenReturn(Runner.DATAFLOW); @@ -318,6 +326,12 @@ public void shouldGroupJobsBySource() throws InvalidProtocolBufferException { when(jobManager.startJob(argThat(new JobMatcher(expectedInput1)))).thenReturn(expected1); when(jobManager.startJob(argThat(new JobMatcher(expectedInput2)))).thenReturn(expected2); when(jobManager.getRunnerType()).thenReturn(Runner.DATAFLOW); + for (FeatureSetProto.FeatureSet fs : Lists.newArrayList(featureSet1, featureSet2)) { + FeatureSetSpec spec = fs.getSpec(); + when(featureSetRepository.findFeatureSetByNameAndProject_NameAndVersion( + spec.getName(), spec.getProject(), spec.getVersion())) + .thenReturn(FeatureSet.fromProto(fs)); + } JobCoordinatorService jcs = new JobCoordinatorService( diff --git a/protos/feast/core/FeatureSet.proto b/protos/feast/core/FeatureSet.proto index f45d47d055d..e7e69ede562 100644 --- a/protos/feast/core/FeatureSet.proto +++ b/protos/feast/core/FeatureSet.proto @@ -69,7 +69,7 @@ message EntitySpec { // Name of the entity. string name = 1; - // Value type of the feature. + // Value type of the entity. feast.types.ValueType.Enum value_type = 2; } From 935602e588f1380f47a5ec2cf628b8bce1175912 Mon Sep 17 00:00:00 2001 From: zhilingc Date: Mon, 4 May 2020 14:14:14 +0800 Subject: [PATCH 11/11] Remove feature and entity references --- .../main/java/feast/core/model/Entity.java | 29 ++++---- .../feast/core/model/EntityReference.java | 72 ------------------- .../main/java/feast/core/model/Feature.java | 23 +++--- .../feast/core/model/FeatureReference.java | 72 ------------------- .../java/feast/core/model/FeatureSet.java | 70 ++++++------------ .../java/feast/core/service/SpecService.java | 2 + 6 files changed, 50 insertions(+), 218 deletions(-) delete mode 100644 core/src/main/java/feast/core/model/EntityReference.java delete mode 100644 core/src/main/java/feast/core/model/FeatureReference.java diff --git a/core/src/main/java/feast/core/model/Entity.java b/core/src/main/java/feast/core/model/Entity.java index 4148b8a2a15..791e280d481 100644 --- a/core/src/main/java/feast/core/model/Entity.java +++ b/core/src/main/java/feast/core/model/Entity.java @@ -19,8 +19,7 @@ import feast.core.FeatureSetProto.EntitySpec; import feast.types.ValueProto.ValueType; import java.util.Objects; -import javax.persistence.EmbeddedId; -import javax.persistence.Table; +import javax.persistence.*; import lombok.Getter; import lombok.Setter; @@ -28,9 +27,17 @@ @Getter @Setter @javax.persistence.Entity -@Table(name = "entities") +@Table( + name = "entities", + uniqueConstraints = @UniqueConstraint(columnNames = {"name", "feature_set_id"})) public class Entity { - @EmbeddedId private EntityReference reference; + + @Id @GeneratedValue private Long id; + + private String name; + + @ManyToOne(fetch = FetchType.LAZY) + private FeatureSet featureSet; /** Data type of the entity. String representation of {@link ValueType} * */ private String type; @@ -38,16 +45,10 @@ public class Entity { public Entity() {} private Entity(String name, ValueType.Enum type) { - this.setReference(new EntityReference(name)); + this.setName(name); this.setType(type.toString()); } - public static Entity withRef(EntityReference entityRef) { - Entity entity = new Entity(); - entity.setReference(entityRef); - return entity; - } - public static Entity fromProto(EntitySpec entitySpec) { Entity entity = new Entity(entitySpec.getName(), entitySpec.getValueType()); return entity; @@ -61,12 +62,12 @@ public boolean equals(Object o) { if (o == null || getClass() != o.getClass()) { return false; } - Entity feature = (Entity) o; - return getReference().equals(feature.getReference()) && getType().equals(feature.getType()); + Entity entity = (Entity) o; + return getName().equals(entity.getName()) && getType().equals(entity.getType()); } @Override public int hashCode() { - return Objects.hash(super.hashCode(), getReference(), getType()); + return Objects.hash(super.hashCode(), getName(), getType()); } } diff --git a/core/src/main/java/feast/core/model/EntityReference.java b/core/src/main/java/feast/core/model/EntityReference.java deleted file mode 100644 index fb05c067c63..00000000000 --- a/core/src/main/java/feast/core/model/EntityReference.java +++ /dev/null @@ -1,72 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * Copyright 2018-2020 The Feast Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package feast.core.model; - -import java.io.Serializable; -import java.util.Objects; -import javax.persistence.Column; -import javax.persistence.Embeddable; -import lombok.AllArgsConstructor; -import lombok.Getter; -import lombok.NoArgsConstructor; -import lombok.Setter; - -@Embeddable -@NoArgsConstructor -@AllArgsConstructor -@Getter -@Setter -public class EntityReference implements Serializable { - // Project the entity belongs to - @Column(nullable = false) - private String project; - - // Feature set the entity belongs to - @Column(name = "feature_set", nullable = false) - private String featureSet; - - // Version of the feature set this entity belongs to - @Column(nullable = false) - private int version; - - // Name of the entity - @Column(nullable = false) - private String name; - - EntityReference(String name) { - this.name = name; - } - - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - EntityReference fieldId = (EntityReference) o; - return Objects.equals(name, fieldId.getName()) - && Objects.equals(project, fieldId.getProject()) - && Objects.equals(featureSet, fieldId.getFeatureSet()); - } - - @Override - public int hashCode() { - return Objects.hash(super.hashCode(), project, featureSet, name); - } -} diff --git a/core/src/main/java/feast/core/model/Feature.java b/core/src/main/java/feast/core/model/Feature.java index 03e91d3dd17..38e2d4549ed 100644 --- a/core/src/main/java/feast/core/model/Feature.java +++ b/core/src/main/java/feast/core/model/Feature.java @@ -34,10 +34,17 @@ @Getter @Setter @Entity -@Table(name = "features") +@Table( + name = "features", + uniqueConstraints = @UniqueConstraint(columnNames = {"name", "feature_set_id"})) public class Feature { - @EmbeddedId private FeatureReference reference; + @Id @GeneratedValue private Long id; + + private String name; + + @ManyToOne(fetch = FetchType.LAZY) + private FeatureSet featureSet; /** Data type of the feature. String representation of {@link ValueType} * */ private String type; @@ -74,16 +81,10 @@ public class Feature { public Feature() {} private Feature(String name, ValueType.Enum type) { - this.setReference(new FeatureReference(name)); + this.setName(name); this.setType(type.toString()); } - public static Feature withRef(FeatureReference featureRef) { - Feature feature = new Feature(); - feature.setReference(featureRef); - return feature; - } - public static Feature fromProto(FeatureSpec featureSpec) { Feature feature = new Feature(featureSpec.getName(), featureSpec.getValueType()); feature.labels = TypeConversion.convertMapToJsonString(featureSpec.getLabelsMap()); @@ -166,7 +167,7 @@ public boolean equals(Object o) { return false; } Feature feature = (Feature) o; - return Objects.equals(getReference(), feature.getReference()) + return Objects.equals(getName(), feature.getName()) && Objects.equals(labels, feature.labels) && Arrays.equals(getPresence(), feature.getPresence()) && Arrays.equals(getGroupPresence(), feature.getGroupPresence()) @@ -188,6 +189,6 @@ public boolean equals(Object o) { @Override public int hashCode() { - return Objects.hash(super.hashCode(), getReference(), getType(), labels); + return Objects.hash(super.hashCode(), getName(), getType(), getLabels()); } } diff --git a/core/src/main/java/feast/core/model/FeatureReference.java b/core/src/main/java/feast/core/model/FeatureReference.java deleted file mode 100644 index bee7a17955d..00000000000 --- a/core/src/main/java/feast/core/model/FeatureReference.java +++ /dev/null @@ -1,72 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * Copyright 2018-2020 The Feast Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package feast.core.model; - -import java.io.Serializable; -import java.util.Objects; -import javax.persistence.Column; -import javax.persistence.Embeddable; -import lombok.AllArgsConstructor; -import lombok.Getter; -import lombok.NoArgsConstructor; -import lombok.Setter; - -@Embeddable -@NoArgsConstructor -@AllArgsConstructor -@Getter -@Setter -public class FeatureReference implements Serializable { - // Project the feature belongs to - @Column(nullable = false) - private String project; - - // Feature set the feature belongs to - @Column(name = "feature_set", nullable = false) - private String featureSet; - - // Version of the feature set this feature belongs to - @Column(nullable = false) - private int version; - - // Name of the feature - @Column(nullable = false) - private String name; - - FeatureReference(String name) { - this.name = name; - } - - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - FeatureReference fieldId = (FeatureReference) o; - return Objects.equals(name, fieldId.getName()) - && Objects.equals(project, fieldId.getProject()) - && Objects.equals(featureSet, fieldId.getFeatureSet()); - } - - @Override - public int hashCode() { - return Objects.hash(super.hashCode(), project, featureSet, name); - } -} diff --git a/core/src/main/java/feast/core/model/FeatureSet.java b/core/src/main/java/feast/core/model/FeatureSet.java index 8ffe334d355..faaee0e41f6 100644 --- a/core/src/main/java/feast/core/model/FeatureSet.java +++ b/core/src/main/java/feast/core/model/FeatureSet.java @@ -39,9 +39,7 @@ public class FeatureSet extends AbstractTimestampEntity implements Comparable { // Id of the featureSet, defined as project/feature_set_name:feature_set_version - @Id - @GeneratedValue(strategy = GenerationType.AUTO) - private int id; + @Id @GeneratedValue private long id; // Name of the featureSet @Column(name = "name", nullable = false) @@ -61,39 +59,19 @@ public class FeatureSet extends AbstractTimestampEntity implements Comparable entities; // Feature fields inside this feature set - @OneToMany(cascade = CascadeType.ALL, fetch = FetchType.EAGER) - @JoinTable( - name = "feature_set_features", - joinColumns = @JoinColumn(name = "feature_set_id"), - inverseJoinColumns = { - @JoinColumn(name = "features_name"), - @JoinColumn(name = "features_project"), - @JoinColumn(name = "features_feature_set_id"), - @JoinColumn(name = "features_version") - }, - indexes = { - @Index( - name = "idx_jobs_feature_set_features_feature_set_id", - columnList = "feature_set_id"), - }) + @OneToMany( + mappedBy = "featureSet", + cascade = CascadeType.ALL, + fetch = FetchType.EAGER, + orphanRemoval = true) private Set features; // Source on which feature rows can be found @@ -189,10 +167,7 @@ public void addEntities(List entities) { } public void addEntity(Entity entity) { - EntityReference entityReference = entity.getReference(); - entityReference.setProject(this.project.getName()); - entityReference.setFeatureSet(this.getName()); - entityReference.setVersion(this.getVersion()); + entity.setFeatureSet(this); entities.add(entity); } @@ -203,10 +178,7 @@ public void addFeatures(List features) { } public void addFeature(Feature feature) { - FeatureReference featureReference = feature.getReference(); - featureReference.setProject(this.project.getName()); - featureReference.setFeatureSet(this.getName()); - featureReference.setVersion(this.getVersion()); + feature.setFeatureSet(this); features.add(feature); } @@ -247,14 +219,14 @@ public FeatureSetProto.FeatureSet toProto() throws InvalidProtocolBufferExceptio private void setEntitySpecFields(EntitySpec.Builder entitySpecBuilder, Entity entityField) { entitySpecBuilder - .setName(entityField.getReference().getName()) + .setName(entityField.getName()) .setValueType(Enum.valueOf(entityField.getType())); } private void setFeatureSpecFields(FeatureSpec.Builder featureSpecBuilder, Feature featureField) throws InvalidProtocolBufferException { featureSpecBuilder - .setName(featureField.getReference().getName()) + .setName(featureField.getName()) .setValueType(Enum.valueOf(featureField.getType())); if (featureField.getPresence() != null) { @@ -331,11 +303,11 @@ public boolean equalTo(FeatureSet other) { Map featuresMap = new HashMap<>(); for (Entity e : entities) { - entitiesMap.putIfAbsent(e.getReference().getName(), e); + entitiesMap.putIfAbsent(e.getName(), e); } for (Feature f : features) { - featuresMap.putIfAbsent(f.getReference().getName(), f); + featuresMap.putIfAbsent(f.getName(), f); } // Ensure map size is consistent with existing fields @@ -348,19 +320,19 @@ public boolean equalTo(FeatureSet other) { // Ensure the other entities and features exist in the field map for (Entity e : other.getEntities()) { - if (!entitiesMap.containsKey(e.getReference().getName())) { + if (!entitiesMap.containsKey(e.getName())) { return false; } - if (!e.equals(entitiesMap.get(e.getReference().getName()))) { + if (!e.equals(entitiesMap.get(e.getName()))) { return false; } } for (Feature f : other.getFeatures()) { - if (!featuresMap.containsKey(f.getReference().getName())) { + if (!featuresMap.containsKey(f.getName())) { return false; } - if (!f.equals(featuresMap.get(f.getReference().getName()))) { + if (!f.equals(featuresMap.get(f.getName()))) { return false; } } diff --git a/core/src/main/java/feast/core/service/SpecService.java b/core/src/main/java/feast/core/service/SpecService.java index 8fec6ac5112..4a068cba353 100644 --- a/core/src/main/java/feast/core/service/SpecService.java +++ b/core/src/main/java/feast/core/service/SpecService.java @@ -33,6 +33,7 @@ import feast.core.CoreServiceProto.UpdateStoreRequest; import feast.core.CoreServiceProto.UpdateStoreResponse; import feast.core.FeatureSetProto; +import feast.core.FeatureSetProto.FeatureSetStatus; import feast.core.SourceProto; import feast.core.StoreProto; import feast.core.StoreProto.Store.Subscription; @@ -335,6 +336,7 @@ public ApplyFeatureSetResponse applyFeatureSet(FeatureSetProto.FeatureSet newFea // Build a new FeatureSet object which includes the new properties FeatureSet featureSet = FeatureSet.fromProto(newFeatureSet); + featureSet.setStatus(FeatureSetStatus.STATUS_PENDING.toString()); if (newFeatureSet.getSpec().getSource() == SourceProto.Source.getDefaultInstance()) { featureSet.setSource(defaultSource); }