Skip to content

Commit 0e2e8ec

Browse files
mrzzyZhu Zhanyan
mrzzy
and
Zhu Zhanyan
authored
Make Projects optional & Update Feature References (feast-dev#693)
* Squash and rebase on master Squashed commits: * Update e2e test verify that users can retrieve from multiple featuresets in batch serving * Add e2e test verify that users can retrieve from multiple featuresets in online serving * Remove and Reserve version field in FeatureReference in ServingService proto * Clarify comment in SpecService docs * Update BQ Retriever to only namespace returned features with featureset only when explictly specified in feature reference. * Configure BQ Retriever SQL queries to namespace returned feature column with feature set name * Resolve unresolved rebase markers in Python's SDK client docs * Squash and Rebase on master * history: * Fix batch e2e failing as new SDK does not accept feature refs with projects. * Fix e2e batch tests: remove rebase markers * Fix keyerror in end to end tests * Fix e2e test that still has removed versions * Correct spelling of omitted in SpecService * Squash and rebase on version removal PR * Fix java lint * Remove comments in JobServiceTest that do not comply with JavaDoc * Clarify naming of unit test in SpecServiceTest * Fix python linting * Refactor AccessManagementService's archiveProject() to use guard clauses * Update Go SDK to strip the project part of string feature references returned from serving * Update Java SDK to strip the project part of string feature references returned from serving * Update Python SDK to strip the project part of string feature references returned from serving * Fix java unit tests afeter adding check for duplicate feature references * Remove trailing whitespace to satisfy python lint. * Fix typo in e2e * Add check that multiple feature references in get online/batch features don't refer to the same feature * Fix end to end tests referencing the same feature in different references * Fix typo in call to get_feature_set in e2e tests. * Fix issue in e2e test where client persisted wrong project. Should used default instead. * Remove support of projects in string Feature References in java SDK * Remove support of projects in string Feature References in go SDK * Remove support of projects in string Feature References in python SDK * Fix python SDK linting failures * Update documentation in CoreService proto to document that field names are unique with a featureset * Update E2E tests to check default project and updated feature reference functionality * Fixed Python SDK's FeatureSetRef rendering with missing project * Make Python's SDK client's archive_project() revert to default project on archive. * Make Feast Core throw an error when the user attempts to archive the default project * Apply spotless formatting to java SDK's RequestUtil * Updated python SDK's client's to parse feature refs without projects and with feature sets * Update Go SDK buildFeatureRefs() to parse feature refs without projects and with feature sets * Regenrate go protos for Go SDK using make compile-protos-go * Changes Java SDK's FeastClient defaultProject to project which overrides project in feature refs. * Updated Java SDK's RequestUtil to parse string feature refs without project/with feature set. * Refactor CachedSpecService to support different variations of feature references. * Update RefUtil's generateFeatureStringRef to FeatureReference with featureset name field * Add missing final to make core Project models's DEFAULT_NAME into a constant. * Added feature_set_name field to ServingService's FeatureReference proto * Remove projects from JobUpdateTask's unit tests * Remove projects from ServingServiceGRpcControllerTest's unit tests. * Remove projects from OnlineServingService's unit tests. * Update CachedSpecService's getFeatureToFeatureSetMapping() to match Feature References with no project. * Update RefUtil's generateFeatureStringRef() to properly render refs without a project * Document in CoreService proto that specifying project in list, get, update featureset is optional * Update SpecService's applyFeatureSet() to auto default project if not specified. * Update SpecService's listFeatureSets() to autofill default project if project unspecified. * Remove comment as FeatureSet's id is no longer a String. * Update SpecService's getFeatureSet() to autofill default project if project unspecified. * Config AccessManagementService to create default project in constructor * Document that FeatureReference's project field is optional. * Allow CachedSpecService's getFeatureSets() to match FeatureReferences with no project by autofilling default project * Make CachedSpecService's featureSetCacheLoader a local var instead of private property as its only used once. * Change SpecServiceTest to use global default project constant. * Make AccessManagementService's archiveProject() throw UnimplementOperation error when trying to archive default project. * Correct incorrect documentation on source being ignored on Core's ApplyFeatureSet * Use ValueProto.Value in java SDK's FeastClient lambda instead of generic Object * Clarify how the ignore project param of feature ref to string() worked in docs in SDKs * Correct missing test to check for empty feature refs in java SDK. * Clarified ref_protos to feature_ref_protos to make code clearer * Rename ServingService's FeatureReference proto 'feature_set_name` to `feature_set` * Remove nested function strip_project() with loop with python SDK's client's get_online_features() * Remove extra version paramter in python sdk test's TestFeatureRef * Change CachedSpecService's "hasProject == true" to more concise "!hasProject" * Removed setFeatureSet() calls to OnlineServingServiceTest as it does not need to be set in the unit test * Make client.set_project() without args reset project to default in python SDK * Updated e2e to test for feature set inference for feature ref without feature set specified * Remove unused field max_age in FeatureReference * Fix typo in e2e * Fix error in e2e tests regarding string splits * Optimise Serving's CachedSpecService populateCache() by directly assigning map. * Remove final in CachedSpecService to allow assignment in featureToFeatureSetMapping * Remove unnescessary copy of list in QueryTemplater Co-authored-by: Zhu Zhanyan <zhu.zhanyan@gojek.com>
1 parent d8459e0 commit 0e2e8ec

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

50 files changed

+1688
-1214
lines changed

‎core/src/main/java/feast/core/grpc/CoreServiceImpl.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,17 @@ public void archiveProject(
183183
accessManagementService.archiveProject(request.getName());
184184
responseObserver.onNext(ArchiveProjectResponse.getDefaultInstance());
185185
responseObserver.onCompleted();
186+
} catch (IllegalArgumentException e) {
187+
log.error("Recieved an invalid request on calling archiveProject method:", e);
188+
responseObserver.onError(
189+
Status.INVALID_ARGUMENT
190+
.withDescription(e.getMessage())
191+
.withCause(e)
192+
.asRuntimeException());
193+
} catch (UnsupportedOperationException e) {
194+
log.error("Attempted to archive an unsupported project:", e);
195+
responseObserver.onError(
196+
Status.UNIMPLEMENTED.withDescription(e.getMessage()).withCause(e).asRuntimeException());
186197
} catch (Exception e) {
187198
log.error("Exception has occurred in the createProject method: ", e);
188199
responseObserver.onError(

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
uniqueConstraints = @UniqueConstraint(columnNames = {"name", "project_name"}))
4040
public class FeatureSet extends AbstractTimestampEntity {
4141

42-
// Id of the featureSet, defined as project/feature_set_name:feature_set_version
4342
@Id @GeneratedValue private long id;
4443

4544
// Name of the featureSet

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,15 @@
2828
@Slf4j
2929
@Service
3030
public class AccessManagementService {
31-
3231
private ProjectRepository projectRepository;
3332

3433
@Autowired
3534
public AccessManagementService(ProjectRepository projectRepository) {
3635
this.projectRepository = projectRepository;
36+
// create default project if it does not yet exist.
37+
if (!projectRepository.existsById(Project.DEFAULT_NAME)) {
38+
this.createProject(Project.DEFAULT_NAME);
39+
}
3740
}
3841

3942
/**
@@ -61,6 +64,9 @@ public void archiveProject(String name) {
6164
if (!project.isPresent()) {
6265
throw new IllegalArgumentException(String.format("Could not find project: \"%s\"", name));
6366
}
67+
if (name.equals(Project.DEFAULT_NAME)) {
68+
throw new UnsupportedOperationException("Archiving the default project is not allowed.");
69+
}
6470
Project p = project.get();
6571
p.setArchived(true);
6672
projectRepository.saveAndFlush(p);

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

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,8 @@ public SpecService(
8080
/**
8181
* Get a feature set matching the feature name and version and project. The feature set name and
8282
* project are required, but version can be omitted by providing 0 for its value. If the version
83-
* is omitted, the latest feature set will be provided.
83+
* is omitted, the latest feature set will be provided. If the project is omitted, the default
84+
* would be used.
8485
*
8586
* @param request: GetFeatureSetRequest Request containing filter parameters.
8687
* @return Returns a GetFeatureSetResponse containing a feature set..
@@ -94,8 +95,9 @@ public GetFeatureSetResponse getFeatureSet(GetFeatureSetRequest request)
9495
if (request.getName().isEmpty()) {
9596
throw new IllegalArgumentException("No feature set name provided");
9697
}
98+
// Autofill default project if project is not specified
9799
if (request.getProject().isEmpty()) {
98-
throw new IllegalArgumentException("No project provided");
100+
request = request.toBuilder().setProject(Project.DEFAULT_NAME).build();
99101
}
100102

101103
FeatureSet featureSet;
@@ -117,7 +119,8 @@ public GetFeatureSetResponse getFeatureSet(GetFeatureSetRequest request)
117119
* projects.
118120
*
119121
* <p>Project name can be explicitly provided, or an asterisk can be provided to match all
120-
* projects. It is not possible to provide a combination of asterisks/wildcards and text.
122+
* projects. It is not possible to provide a combination of asterisks/wildcards and text. If the
123+
* project name is omitted, the default project would be used.
121124
*
122125
* <p>The feature set name in the filter accepts an asterisk as a wildcard. All matching feature
123126
* sets will be returned. Regex is not supported. Explicitly defining a feature set name is not
@@ -131,14 +134,19 @@ public ListFeatureSetsResponse listFeatureSets(ListFeatureSetsRequest.Filter fil
131134
String name = filter.getFeatureSetName();
132135
String project = filter.getProject();
133136

134-
if (project.isEmpty() || name.isEmpty()) {
137+
if (name.isEmpty()) {
135138
throw new IllegalArgumentException(
136-
"Invalid listFeatureSetRequest, missing arguments. Must provide project and feature set name.");
139+
"Invalid listFeatureSetRequest, missing arguments. Must provide feature set name:");
137140
}
138141

139142
checkValidCharactersAllowAsterisk(name, "featureSetName");
140143
checkValidCharactersAllowAsterisk(project, "projectName");
141144

145+
// Autofill default project if project not specified
146+
if (project.isEmpty()) {
147+
project = Project.DEFAULT_NAME;
148+
}
149+
142150
List<FeatureSet> featureSets = new ArrayList<FeatureSet>() {};
143151

144152
if (project.contains("*")) {
@@ -227,12 +235,21 @@ public ListStoresResponse listStores(ListStoresRequest.Filter filter) {
227235
*
228236
* <p>This function is idempotent. If no changes are detected in the incoming featureSet's schema,
229237
* this method will update the incoming featureSet spec with the latest version stored in the
230-
* repository, and return that.
238+
* repository, and return that. If project is not specified in the given featureSet, will assign
239+
* the featureSet to the'default' project.
231240
*
232241
* @param newFeatureSet Feature set that will be created or updated.
233242
*/
234243
public ApplyFeatureSetResponse applyFeatureSet(FeatureSetProto.FeatureSet newFeatureSet)
235244
throws InvalidProtocolBufferException {
245+
// Autofill default project if not specified
246+
if (newFeatureSet.getSpec().getProject().isEmpty()) {
247+
newFeatureSet =
248+
newFeatureSet
249+
.toBuilder()
250+
.setSpec(newFeatureSet.getSpec().toBuilder().setProject(Project.DEFAULT_NAME).build())
251+
.build();
252+
}
236253

237254
// Validate incoming feature set
238255
FeatureSetValidator.validateSpec(newFeatureSet);

‎core/src/test/java/feast/core/job/JobUpdateTaskTest.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,7 @@ public class JobUpdateTaskTest {
5353

5454
private static final FeatureSetProto.FeatureSet.Builder fsBuilder =
5555
FeatureSetProto.FeatureSet.newBuilder().setMeta(FeatureSetMeta.newBuilder());
56-
private static final FeatureSetSpec.Builder specBuilder =
57-
FeatureSetSpec.newBuilder().setProject("project1");
56+
private static final FeatureSetSpec.Builder specBuilder = FeatureSetSpec.newBuilder();
5857

5958
@Mock private JobManager jobManager;
6059

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

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
* Copyright 2018-2019 The Feast Authors
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* https://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package feast.core.service;
18+
19+
import static org.mockito.Mockito.verify;
20+
import static org.mockito.Mockito.when;
21+
import static org.mockito.MockitoAnnotations.initMocks;
22+
23+
import feast.core.dao.ProjectRepository;
24+
import feast.core.model.Project;
25+
import java.util.Optional;
26+
import org.junit.Before;
27+
import org.junit.Rule;
28+
import org.junit.Test;
29+
import org.junit.rules.ExpectedException;
30+
import org.mockito.Mock;
31+
32+
public class AccessManagementServiceTest {
33+
@Rule public ExpectedException expectedException = ExpectedException.none();
34+
// mocks
35+
@Mock private ProjectRepository projectRepository;
36+
// dummy models
37+
private Project defaultProject;
38+
private Project testProject;
39+
40+
// test target
41+
private AccessManagementService accessService;
42+
43+
@Before
44+
public void setup() {
45+
initMocks(this);
46+
// setup dummy models for testing
47+
this.defaultProject = new Project(Project.DEFAULT_NAME);
48+
this.testProject = new Project("project");
49+
// setup test target
50+
when(this.projectRepository.existsById(Project.DEFAULT_NAME)).thenReturn(false);
51+
this.accessService = new AccessManagementService(this.projectRepository);
52+
}
53+
54+
@Test
55+
public void testDefaultProjectCreateInConstructor() {
56+
verify(this.projectRepository).saveAndFlush(this.defaultProject);
57+
}
58+
59+
@Test
60+
public void testArchiveProject() {
61+
when(this.projectRepository.findById("project")).thenReturn(Optional.of(this.testProject));
62+
this.accessService.archiveProject("project");
63+
this.testProject.setArchived(true);
64+
verify(this.projectRepository).saveAndFlush(this.testProject);
65+
// reset archived flag
66+
this.testProject.setArchived(false);
67+
}
68+
69+
@Test
70+
public void shouldNotArchiveDefaultProject() {
71+
expectedException.expect(IllegalArgumentException.class);
72+
this.accessService.archiveProject(Project.DEFAULT_NAME);
73+
}
74+
}

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

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@ public class JobServiceTest {
6969
// test target
7070
public JobService jobService;
7171

72-
/* unit test setup */
7372
@Before
7473
public void setup() {
7574
initMocks(this);
@@ -107,7 +106,6 @@ public void setup() {
107106
new JobService(this.jobRepository, this.specService, Arrays.asList(this.jobManager));
108107
}
109108

110-
// setup fake spec service
111109
public void setupSpecService() {
112110
try {
113111
ListFeatureSetsResponse response =
@@ -124,7 +122,6 @@ public void setupSpecService() {
124122
}
125123
}
126124

127-
// setup fake job repository
128125
public void setupJobRepository() {
129126
when(this.jobRepository.findById(this.job.getId())).thenReturn(Optional.of(this.job));
130127
when(this.jobRepository.findByStoreName(this.dataStore.getName()))
@@ -134,14 +131,12 @@ public void setupJobRepository() {
134131
when(this.jobRepository.findAll()).thenReturn(Arrays.asList(this.job));
135132
}
136133

137-
// TODO: setup fake job manager
138134
public void setupJobManager() {
139135
when(this.jobManager.getRunnerType()).thenReturn(Runner.DATAFLOW);
140136
when(this.jobManager.restartJob(this.job))
141137
.thenReturn(this.newDummyJob(this.job.getId(), this.job.getExtId(), JobStatus.PENDING));
142138
}
143139

144-
// dummy model constructorss
145140
private FeatureSet newDummyFeatureSet(String name, int version, String project) {
146141
Feature feature = TestObjectFactory.CreateFeature(name + "_feature", Enum.INT64);
147142
Entity entity = TestObjectFactory.CreateEntity(name + "_entity", Enum.STRING);
@@ -203,7 +198,6 @@ private List<ListFeatureSetsRequest.Filter> newDummyListRequestFilters() {
203198
.build());
204199
}
205200

206-
/* unit tests */
207201
private ListIngestionJobsResponse tryListJobs(ListIngestionJobsRequest request) {
208202
ListIngestionJobsResponse response = null;
209203
try {
@@ -216,7 +210,6 @@ private ListIngestionJobsResponse tryListJobs(ListIngestionJobsRequest request)
216210
return response;
217211
}
218212

219-
// list jobs
220213
@Test
221214
public void testListJobsById() {
222215
ListIngestionJobsRequest.Filter filter =
@@ -275,7 +268,6 @@ public void testListIngestionJobByFeatureSetReference() {
275268
assertThat(this.tryListJobs(request).getJobs(0), equalTo(this.ingestionJob));
276269
}
277270

278-
// stop jobs
279271
private StopIngestionJobResponse tryStopJob(
280272
StopIngestionJobRequest request, boolean expectError) {
281273
StopIngestionJobResponse response = null;

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

Lines changed: 40 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/*
22
* SPDX-License-Identifier: Apache-2.0
3-
* Copyright 2018-2019 The Feast Authors
3+
* Copyright 2018-2020 The Feast Authors
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
66
* you may not use this file except in compliance with the License.
@@ -34,6 +34,7 @@
3434
import feast.proto.core.CoreServiceProto.ApplyFeatureSetResponse;
3535
import feast.proto.core.CoreServiceProto.ApplyFeatureSetResponse.Status;
3636
import feast.proto.core.CoreServiceProto.GetFeatureSetRequest;
37+
import feast.proto.core.CoreServiceProto.GetFeatureSetResponse;
3738
import feast.proto.core.CoreServiceProto.ListFeatureSetsRequest.Filter;
3839
import feast.proto.core.CoreServiceProto.ListFeatureSetsResponse;
3940
import feast.proto.core.CoreServiceProto.ListStoresRequest;
@@ -105,11 +106,13 @@ public void setUp() {
105106
Feature f3f1 = TestObjectFactory.CreateFeature("f3f1", Enum.INT64);
106107
Feature f3f2 = TestObjectFactory.CreateFeature("f3f2", Enum.INT64);
107108
Entity f3e1 = TestObjectFactory.CreateEntity("f3e1", Enum.STRING);
108-
FeatureSet featureSet3v1 =
109+
FeatureSet featureSet3 =
109110
TestObjectFactory.CreateFeatureSet(
110111
"f3", "project1", Arrays.asList(f3e1), Arrays.asList(f3f2, f3f1));
111112

112-
featureSets = Arrays.asList(featureSet1, featureSet2);
113+
FeatureSet featureSet4 = newDummyFeatureSet("f4", Project.DEFAULT_NAME);
114+
featureSets = Arrays.asList(featureSet1, featureSet2, featureSet3, featureSet4);
115+
113116
when(featureSetRepository.findAll()).thenReturn(featureSets);
114117
when(featureSetRepository.findAllByOrderByNameAsc()).thenReturn(featureSets);
115118
when(featureSetRepository.findFeatureSetByNameAndProject_Name("f1", "project1"))
@@ -160,15 +163,6 @@ public void shouldGetAllFeatureSetsIfOnlyWildcardsProvided()
160163
assertThat(actual, equalTo(expected));
161164
}
162165

163-
@Test
164-
public void listFeatureSetShouldFailIfFeatureSetProvidedWithoutProject()
165-
throws InvalidProtocolBufferException {
166-
expectedException.expect(IllegalArgumentException.class);
167-
expectedException.expectMessage(
168-
"Invalid listFeatureSetRequest, missing arguments. Must provide project and feature set name.");
169-
specService.listFeatureSets(Filter.newBuilder().setFeatureSetName("f1").build());
170-
}
171-
172166
@Test
173167
public void shouldGetAllFeatureSetsMatchingNameWithWildcardSearch()
174168
throws InvalidProtocolBufferException {
@@ -511,6 +505,26 @@ public void applyFeatureSetShouldCreateProjectWhenNotAlreadyExists()
511505
equalTo(incomingFeatureSet.getSpec().getProject()));
512506
}
513507

508+
@Test
509+
public void applyFeatureSetShouldUsedDefaultProjectIfUnspecified()
510+
throws InvalidProtocolBufferException {
511+
Feature f3f1 = TestObjectFactory.CreateFeature("f3f1", Enum.INT64);
512+
Feature f3f2 = TestObjectFactory.CreateFeature("f3f2", Enum.INT64);
513+
Entity f3e1 = TestObjectFactory.CreateEntity("f3e1", Enum.STRING);
514+
515+
// In protov3, unspecified project defaults to ""
516+
FeatureSetProto.FeatureSet incomingFeatureSet =
517+
TestObjectFactory.CreateFeatureSet("f3", "", Arrays.asList(f3e1), Arrays.asList(f3f2, f3f1))
518+
.toProto();
519+
ApplyFeatureSetResponse applyFeatureSetResponse =
520+
specService.applyFeatureSet(incomingFeatureSet);
521+
assertThat(applyFeatureSetResponse.getStatus(), equalTo(Status.CREATED));
522+
523+
assertThat(
524+
applyFeatureSetResponse.getFeatureSet().getSpec().getProject(),
525+
equalTo(Project.DEFAULT_NAME));
526+
}
527+
514528
@Test
515529
public void applyFeatureSetShouldFailWhenProjectIsArchived()
516530
throws InvalidProtocolBufferException {
@@ -661,10 +675,20 @@ public void shouldDoNothingIfNoChange() throws InvalidProtocolBufferException {
661675
}
662676

663677
@Test
664-
public void shouldFailIfGetFeatureSetWithoutProject() throws InvalidProtocolBufferException {
665-
expectedException.expect(IllegalArgumentException.class);
666-
expectedException.expectMessage("No project provided");
667-
specService.getFeatureSet(GetFeatureSetRequest.newBuilder().setName("f1").build());
678+
public void getOrListFeatureSetShouldUseDefaultProjectIfProjectUnspecified()
679+
throws InvalidProtocolBufferException {
680+
when(featureSetRepository.findFeatureSetByNameAndProject_Name("f4", Project.DEFAULT_NAME))
681+
.thenReturn(featureSets.get(3));
682+
FeatureSet expected = featureSets.get(3);
683+
// check getFeatureSet()
684+
GetFeatureSetResponse getResponse =
685+
specService.getFeatureSet(GetFeatureSetRequest.newBuilder().setName("f4").build());
686+
assertThat(getResponse.getFeatureSet(), equalTo(expected.toProto()));
687+
688+
// check listFeatureSets()
689+
ListFeatureSetsResponse listResponse =
690+
specService.listFeatureSets(Filter.newBuilder().setFeatureSetName("f4").build());
691+
assertThat(listResponse.getFeatureSetsList(), equalTo(Arrays.asList(expected.toProto())));
668692
}
669693

670694
private FeatureSet newDummyFeatureSet(String name, String project) {

0 commit comments

Comments
 (0)