Skip to content

Enforce JDK 11 for development, but build Ingestion to target Java 8 #518

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
feast-ci-bot merged 2 commits into from
Mar 28, 2020
Merged

Enforce JDK 11 for development, but build Ingestion to target Java 8 #518

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
6ac945d
Require Java 11 with Enforcer
ches Mar 7, 2020
c6d768c
Build Ingestion to target Java 8, for Beam compat
ches Mar 7, 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
9 changes: 9 additions & 0 deletions datatypes/java/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,15 @@

<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: 33 additions & 1 deletion docs/contributing/development-guide.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ The following software is required for Feast development

* Java SE Development Kit 11
* Python version 3.6 \(or above\) and pip
* [Maven ](https://maven.apache.org/install.html)version 3.6.x
* [Maven](https://maven.apache.org/install.html) version 3.6.x

Additionally, [grpc\_cli](https://github.com/grpc/grpc/blob/master/doc/command_line_tool.md) is useful for debugging and quick testing of gRPC endpoints.

Expand Down Expand Up @@ -444,3 +444,35 @@ If you have made it to this point successfully you should have a functioning Fea

It is important to note that most of the functionality demonstrated above is already available in a more abstracted form in the Python SDK \(Feast management, data ingestion, feature retrieval\) and the Java/Go SDKs \(feature retrieval\). However, it is useful to understand these internals from a development standpoint.

### 5 Appendix

#### 5.1 Java / JDK Versions

Feast requires a Java 11 or greater JDK for building the project. This is checked by Maven so you'll be informed if you try to use an older version.

Leaf application modules of the build such as the Core and Serving APIs compile with [the `javac --release` flag] set for 11, and Ingestion and shared library modules that it uses target release 8. Here's why.

While we want to take advantage of advancements in the (long-term supported) language and platform, and for contributors to enjoy those as well, Apache Beam forms a major part of Feast's Ingestion component and fully validating Beam on Java 11 [is an open issue][BEAM-2530]. Moreover, Beam runners other than the DirectRunner may lag behind further still—Spark does not _build or run_ on Java 11 until its version 3.0 which is still in preview. Presumably Beam's SparkRunner will have to wait for Spark, and for its implementation to update to Spark 3.

To have confidence in Beam stability and our users' ability to deploy Feast on a range of Beam runners, we will continue to target Java 8 bytecode and Platform version for Feast Ingestion, until the ecosystem moves forward.

You do _not_ need a Java 8 SDK installed for development. Newer JDKs can build for the older platform, and Feast's Maven build does this automatically.

See [Feast issue #517][\#517] for discussion.

[the `javac --release` flag]: https://stackoverflow.com/questions/43102787/what-is-the-release-flag-in-the-java-9-compiler
[BEAM-2530]: https://issues.apache.org/jira/browse/BEAM-2530
[\#517]: https://github.com/gojek/feast/issues/517

#### 5.2 IntelliJ Tips and Troubleshooting

For IntelliJ users, this section collects notes and setup recommendations for working comfortably on the Feast project, especially to coexist as peacefully as possible with the Maven build.
Copy link
Copy Markdown
Member Author

@ches ches Mar 28, 2020

Choose a reason for hiding this comment

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

I'll move some other IntelliJ advice here that's duplicated (and out of sync) between the Contributing and Development guides, in another PR or GitBook edit.

I believe the thinking was that Contributing was getting unwieldy with technical setup / dev environment verification—I would agree, will take a look at moving things over with references left in Contributing. Probably this is also a legacy of duplicate CONTRIBUTING.md kept in the root awhile for GitHub.

Copy link
Copy Markdown
Member

@woop woop Mar 28, 2020

Choose a reason for hiding this comment

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

Yip, your assessment is correct. Lot's of little pebbles here, as you call them. We can chip away at it over time.


##### Language Level

IntelliJ uses a notion of "Language Level" to drive assistance features according to the target Java version of a project. It often infers this appropriately from a Maven import, but it's wise to check, especially for a multi-module project with differing target Java releases across modules, like ours. Language level [can be set per module][module lang level]—if IntelliJ is suggesting things that turn out to fail when building with `mvn`, make sure the language level of the module in question corresponds to the `<release>` set for `maven-compiler-plugin` in the module's `pom.xml` (11 is project default if the module doesn't set one).

Ensure that the Project _SDK_ (not language level) is set to a Java 11 JDK, and that all modules use the Project SDK (see [the Dependencies tab] in the Modules view).

[module lang level]: https://www.jetbrains.com/help/idea/sources-tab.html#module_language_level
[the Dependencies tab]: https://www.jetbrains.com/help/idea/dependencies.html
9 changes: 9 additions & 0 deletions ingestion/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,15 @@

<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
4 changes: 2 additions & 2 deletions ...ion/src/test/java/feast/ingestion/transform/metrics/WriteFeatureValueMetricsDoFnTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ private Map<String, Iterable<FeatureRow>> readTestInput(String path) throws IOEx
}
List<String> colNames = new ArrayList<>();
for (String line : lines) {
if (line.strip().length() < 1) {
if (line.trim().length() < 1) {
continue;
}
String[] splits = line.split(",");
Expand All @@ -156,7 +156,7 @@ private Map<String, Iterable<FeatureRow>> readTestInput(String path) throws IOEx

Builder featureRowBuilder = FeatureRow.newBuilder();
for (int i = 0; i < splits.length; i++) {
String colVal = splits[i].strip();
String colVal = splits[i].trim();
if (i == 0) {
featureRowBuilder.setFeatureSet(colVal);
continue;
Expand Down
23 changes: 13 additions & 10 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,6 @@
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-javadoc-plugin</artifactId>
<version>3.1.1</version>
<executions>
<execution>
<id>attach-javadocs</id>
Expand Down Expand Up @@ -408,15 +407,8 @@
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<version>3.8.1</version>
<configuration>
<release>11</release>
<compilerArgs>
<arg>-Xlint:all</arg>
<!-- -Xdoclint:all breaks on a false positive in JDK < 9 -->
<!-- https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8186647 -->
<arg>-Xdoclint:-syntax</arg>
</compilerArgs>
</configuration>
</plugin>
<plugin>
Expand All @@ -442,7 +434,7 @@
<version>[3.6,4.0)</version>
</requireMavenVersion>
<requireJavaVersion>
<version>[1.8,11.1)</version>
<version>[11.0,)</version>
</requireJavaVersion>
<reactorModuleConvergence />
</rules>
Expand Down Expand Up @@ -574,6 +566,17 @@
<artifactId>docker-maven-plugin</artifactId>
<version>0.20.1</version>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<version>3.8.1</version>
<configuration>
<compilerArgs>
<arg>-Xlint:all</arg>
<arg>-Xdoclint:all</arg>
</compilerArgs>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-dependency-plugin</artifactId>
Expand All @@ -591,7 +594,7 @@
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-javadoc-plugin</artifactId>
<version>3.1.0</version>
<version>3.1.1</version>
</plugin>
<plugin>
<groupId>org.codehaus.mojo</groupId>
Expand Down