Skip to content

Commit

Permalink
Restore Feast Java SDK and Ingestion compatibility with Java 8 runtim…
Browse files Browse the repository at this point in the history
…es (#722)

* Make storage modules target Java 8, since Ingestion must

With the addition of the Storage APIs and connector implementations, we
did not specify that their build should target Java 8 bytecode. Because
Ingestion depends on them, we regressed on keeping Ingestion compatible
with Java 8 runtimes according to #518.

This could be confirmed prior to this change with, for example:

    $ mvn -pl ingestion clean compile
    $ javap -v storage/connectors/redis/target/classes/feast/storage/connectors/redis/writer/RedisCustomIO.class | grep major
      major version: 55

I take this as proof that a blacklist approach with 11 as the default in
our root POM is error prone, so this change flips it so that 8 is
default and our leaf applications that are able use 11 opt into it in
whitelist fashion.

* Target Java 8 compatibility for Java SDK
  • Loading branch information
ches authored May 22, 2020
1 parent ec7d413 commit 8538769
Show file tree
Hide file tree
Showing 10 changed files with 63 additions and 32 deletions.
8 changes: 8 additions & 0 deletions core/pom.xml
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
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
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
-->
<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
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,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
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
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
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
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
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
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

0 comments on commit 8538769

Please sign in to comment.