Skip to content

Restore Feast Java SDK and Ingestion compatibility with Java 8 runtimes #722

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
woop merged 2 commits into from
May 22, 2020
Merged

Restore Feast Java SDK and Ingestion compatibility with Java 8 runtimes #722

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7b1e820
Make storage modules target Java 8, since Ingestion must
ches May 21, 2020
2e0c16a
Target Java 8 compatibility for Java SDK
ches May 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions core/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,14 @@

<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<release>11</release>
</configuration>
</plugin>

<plugin>
<groupId>org.jacoco</groupId>
<artifactId>jacoco-maven-plugin</artifactId>
Expand Down
9 changes: 0 additions & 9 deletions datatypes/java/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,6 @@

<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<!-- To ensure Ingestion remains compatible for Java 8-only Beam runners -->
<release>8</release>
</configuration>
</plugin>

<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-dependency-plugin</artifactId>
Expand Down
34 changes: 25 additions & 9 deletions ingestion/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,6 @@

<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<!-- To ensure Ingestion remains compatible for Java 8-only Beam runners -->
<release>8</release>
</configuration>
</plugin>

<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-shade-plugin</artifactId>
Expand Down Expand Up @@ -84,6 +75,31 @@
</executions>
</plugin>

<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-enforcer-plugin</artifactId>
<executions>
<!--
We want to keep Ingestion compatible with Java 8 runtime for Beam runners, #518.
Unfortunately this doesn't seem to work on Reactor modules, though:
https://github.com/mojohaus/mojo-parent/issues/85
Comment on lines 83 to 85
Copy link
Copy Markdown
Member Author

@ches ches May 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hoped that this would fail before applying the fix, verifying that it could catch this problem, but it didn't. I kept this in, because if it does fail on third-party dependencies, we would probably like to know if/when it changes in the future. This rule is from extra-enforcer-rules which was added #451 but not actually used for anything so far, I think.

-->
<execution>
<id>enforce-bytecode-version</id>
<goals>
<goal>enforce</goal>
</goals>
<configuration>
<rules>
<enforceBytecodeVersion>
<maxJdkVersion>1.8</maxJdkVersion>
</enforceBytecodeVersion>
</rules>
</configuration>
</execution>
</executions>
</plugin>

<plugin>
<groupId>org.jacoco</groupId>
<artifactId>jacoco-maven-plugin</artifactId>
Expand Down
3 changes: 2 additions & 1 deletion pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,8 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<release>11</release>
<!-- Our leaf applications target 11, except Ingestion and its deps remain on 8 for Beam -->
<release>8</release>
</configuration>
</plugin>
<plugin>
Expand Down
3 changes: 2 additions & 1 deletion sdk/java/src/test/java/com/gojek/feast/RequestUtilTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;

import com.google.common.collect.ImmutableList;
import com.google.protobuf.TextFormat;
import feast.proto.serving.ServingAPIProto.FeatureReference;
import java.util.Arrays;
Expand Down Expand Up @@ -82,7 +83,7 @@ void renderFeatureRef_ShouldReturnFeatureRefString(
}

private static Stream<Arguments> provideInvalidFeatureRefs() {
return Stream.of(Arguments.of(List.of("project/feature", "")));
return Stream.of(Arguments.of(ImmutableList.of("project/feature", "")));
}

@ParameterizedTest
Expand Down
8 changes: 8 additions & 0 deletions serving/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,14 @@

<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<release>11</release>
</configuration>
</plugin>

<plugin>
<groupId>org.jacoco</groupId>
<artifactId>jacoco-maven-plugin</artifactId>
Expand Down
6 changes: 4 additions & 2 deletions ...e/connectors/redis/src/main/java/feast/storage/connectors/redis/writer/RedisCustomIO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,10 @@ public class RedisCustomIO {
private static final int DEFAULT_BATCH_SIZE = 1000;
private static final int DEFAULT_TIMEOUT = 2000;

private static TupleTag<FeatureRow> successfulInsertsTag = new TupleTag<>("successfulInserts") {};
private static TupleTag<FailedElement> failedInsertsTupleTag = new TupleTag<>("failedInserts") {};
private static TupleTag<FeatureRow> successfulInsertsTag =
new TupleTag<FeatureRow>("successfulInserts") {};
private static TupleTag<FailedElement> failedInsertsTupleTag =
new TupleTag<FailedElement>("failedInserts") {};

private static final Logger log = LoggerFactory.getLogger(RedisCustomIO.class);

Expand Down
8 changes: 4 additions & 4 deletions ...edis/src/test/java/feast/storage/connectors/redis/retriever/RedisOnlineRetrieverTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ public void shouldReturnResponseWithValuesIfKeysPresent() {
when(syncCommands.mget(redisKeyList)).thenReturn(featureRowBytes);

List<List<FeatureRow>> expected =
List.of(
ImmutableList.of(
Lists.newArrayList(
FeatureRow.newBuilder()
.setEventTimestamp(Timestamp.newBuilder().setSeconds(100))
Expand All @@ -153,7 +153,7 @@ public void shouldReturnResponseWithValuesIfKeysPresent() {
.build()));

List<List<FeatureRow>> actual =
redisOnlineRetriever.getOnlineFeatures(entityRows, List.of(featureSetRequest));
redisOnlineRetriever.getOnlineFeatures(entityRows, ImmutableList.of(featureSetRequest));
assertThat(actual, equalTo(expected));
}

Expand Down Expand Up @@ -201,7 +201,7 @@ public void shouldReturnResponseWithUnsetValuesIfKeysNotPresent() {
when(syncCommands.mget(redisKeyList)).thenReturn(featureRowBytes);

List<List<FeatureRow>> expected =
List.of(
ImmutableList.of(
Lists.newArrayList(
FeatureRow.newBuilder()
.setEventTimestamp(Timestamp.newBuilder().setSeconds(100))
Expand All @@ -220,7 +220,7 @@ public void shouldReturnResponseWithUnsetValuesIfKeysNotPresent() {
.build()));

List<List<FeatureRow>> actual =
redisOnlineRetriever.getOnlineFeatures(entityRows, List.of(featureSetRequest));
redisOnlineRetriever.getOnlineFeatures(entityRows, ImmutableList.of(featureSetRequest));
assertThat(actual, equalTo(expected));
}

Expand Down
6 changes: 4 additions & 2 deletions ...ster/src/main/java/feast/storage/connectors/rediscluster/writer/RedisClusterCustomIO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,10 @@ public class RedisClusterCustomIO {
private static final int DEFAULT_BATCH_SIZE = 1000;
private static final int DEFAULT_TIMEOUT = 2000;

private static TupleTag<FeatureRow> successfulInsertsTag = new TupleTag<>("successfulInserts") {};
private static TupleTag<FailedElement> failedInsertsTupleTag = new TupleTag<>("failedInserts") {};
private static TupleTag<FeatureRow> successfulInsertsTag =
new TupleTag<FeatureRow>("successfulInserts") {};
private static TupleTag<FailedElement> failedInsertsTupleTag =
new TupleTag<FailedElement>("failedInserts") {};

private static final Logger log = LoggerFactory.getLogger(RedisClusterCustomIO.class);

Expand Down
10 changes: 6 additions & 4 deletions ...java/feast/storage/connectors/rediscluster/retriever/RedisClusterOnlineRetrieverTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ public void shouldReturnResponseWithValuesIfKeysPresent() {
when(syncCommands.mget(redisKeyList)).thenReturn(featureRowBytes);

List<List<FeatureRow>> expected =
List.of(
ImmutableList.of(
Lists.newArrayList(
FeatureRow.newBuilder()
.setEventTimestamp(Timestamp.newBuilder().setSeconds(100))
Expand All @@ -153,7 +153,8 @@ public void shouldReturnResponseWithValuesIfKeysPresent() {
.build()));

List<List<FeatureRow>> actual =
redisClusterOnlineRetriever.getOnlineFeatures(entityRows, List.of(featureSetRequest));
redisClusterOnlineRetriever.getOnlineFeatures(
entityRows, ImmutableList.of(featureSetRequest));
assertThat(actual, equalTo(expected));
}

Expand Down Expand Up @@ -201,7 +202,7 @@ public void shouldReturnResponseWithUnsetValuesIfKeysNotPresent() {
when(syncCommands.mget(redisKeyList)).thenReturn(featureRowBytes);

List<List<FeatureRow>> expected =
List.of(
ImmutableList.of(
Lists.newArrayList(
FeatureRow.newBuilder()
.setEventTimestamp(Timestamp.newBuilder().setSeconds(100))
Expand All @@ -220,7 +221,8 @@ public void shouldReturnResponseWithUnsetValuesIfKeysNotPresent() {
.build()));

List<List<FeatureRow>> actual =
redisClusterOnlineRetriever.getOnlineFeatures(entityRows, List.of(featureSetRequest));
redisClusterOnlineRetriever.getOnlineFeatures(
entityRows, ImmutableList.of(featureSetRequest));
assertThat(actual, equalTo(expected));
}

Expand Down