Skip to content
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

Jump to bottom

Clean up the Maven build #262

Merged
merged 5 commits into from
Oct 21, 2019
Merged

Clean up the Maven build #262

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
aaaf997
Clean up the Maven build
ches Oct 19, 2019
88b3fcd
Enable -Xlint warnings, fix trivial deprecations
ches Oct 19, 2019
d4a318e
Make sdk/java a child module of the main build
ches Oct 19, 2019
e97bf59
Don't recompile protos when they haven't changed
ches Oct 19, 2019
5e65a07
Remove unused dependencies
ches Oct 20, 2019
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
206 changes: 11 additions & 195 deletions core/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,15 @@
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
<modelVersion>4.0.0</modelVersion>

<parent>
<groupId>feast</groupId>
<artifactId>feast-parent</artifactId>
<version>${revision}</version>
</parent>
<artifactId>feast-core</artifactId>
<packaging>jar</packaging>

<name>Feast Core</name>
<artifactId>feast-core</artifactId>

<build>
<plugins>
Expand Down Expand Up @@ -78,46 +79,32 @@
</plugins>
</build>

<repositories>
<repository>
<id>jcenter</id>
<url>https://jcenter.bintray.com/</url>
</repository>
<repository>
<id>jitpack.io</id>
<url>https://jitpack.io</url>
</repository>
</repositories>

<dependencies>
<dependency>
<groupId>com.github.gojek.feast</groupId>
<groupId>feast</groupId>
<artifactId>feast-ingestion</artifactId>
<version>9428b230a5</version>
<version>${project.version}</version>
Comment on lines -94 to +86
Copy link
Member Author

Choose a reason for hiding this comment

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

Theme #1

</dependency>

<dependency>
<groupId>javax.inject</groupId>
<artifactId>javax.inject</artifactId>
<version>1</version>
</dependency>
<!--compile "org.springframework.boot:spring-boot-starter-web:${springBootVersion}"-->
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-web</artifactId>
<version>${springBootVersion}</version>
<exclusions>
<exclusion>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-logging</artifactId>
</exclusion>
</exclusions>
</dependency>
<!--compile 'org.springframework.boot:spring-boot-starter-log4j2'-->
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-log4j2</artifactId>
<version>${springBootVersion}</version>
</dependency>
<!--compile 'org.lognet:grpc-spring-boot-starter:2.4.1'-->
<dependency>
<groupId>org.lognet</groupId>
<artifactId>grpc-spring-boot-starter</artifactId>
<version>2.4.1</version>
</dependency>
<!--compile "org.springframework.boot:spring-boot-starter-data-jpa:${springBootVersion}"-->
<dependency>
Expand All @@ -129,66 +116,42 @@
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-actuator</artifactId>
<version>${springBootVersion}</version>
</dependency>

<!-- https://mvnrepository.com/artifact/org.springframework.boot/spring-boot-configuration-processor -->
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-configuration-processor</artifactId>
<version>${springBootVersion}</version>
</dependency>


<!--compile "io.grpc:grpc-netty:${grpcVersion}"-->
<dependency>
<groupId>io.grpc</groupId>
<artifactId>grpc-netty</artifactId>
<version>${grpcVersion}</version>
</dependency>
<!--compile "io.grpc:grpc-services:${grpcVersion}"-->
<dependency>
<groupId>io.grpc</groupId>
<artifactId>grpc-services</artifactId>
<version>${grpcVersion}</version>
</dependency>
<!--compile "io.grpc:grpc-stub:${grpcVersion}"-->
<dependency>
<groupId>io.grpc</groupId>
<artifactId>grpc-stub</artifactId>
<version>${grpcVersion}</version>
</dependency>
<!--compile "com.google.protobuf:protobuf-java-util:${protobufVersion}"-->
<dependency>
<groupId>com.google.protobuf</groupId>
<artifactId>protobuf-java-util</artifactId>
<version>${protobufVersion}</version>
</dependency>

<!--compile 'com.google.guava:guava:26.0-jre'-->
<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
<version>26.0-jre</version>
</dependency>
<!--compile 'com.google.code.gson:gson:2.8.5'-->
<dependency>
<groupId>com.google.code.gson</groupId>
<artifactId>gson</artifactId>
<version>2.8.5</version>
</dependency>
<!--compile 'commons-codec:commons-codec:1.10'-->
<dependency>
<groupId>commons-codec</groupId>
<artifactId>commons-codec</artifactId>
<version>1.10</version>
</dependency>
<!--compile 'joda-time:joda-time:2.9.9'-->
<dependency>
<groupId>joda-time</groupId>
<artifactId>joda-time</artifactId>
<version>2.9.9</version>
</dependency>
<dependency>
<groupId>com.google.api-client</groupId>
<artifactId>google-api-client</artifactId>
Expand All @@ -200,153 +163,28 @@
<version>v1b3-rev266-1.25.0</version>
</dependency>

<!--compile 'org.jdbi:jdbi3:3.0.0-beta2'-->
<dependency>
<groupId>org.jdbi</groupId>
<artifactId>jdbi3</artifactId>
<version>3.0.0-beta2</version>
</dependency>
<!--compile 'org.jdbi:jdbi3-sqlobject:3.4.0'-->
<dependency>
<groupId>org.jdbi</groupId>
<artifactId>jdbi3-sqlobject</artifactId>
<version>3.4.0</version>
</dependency>
<!--compile 'org.postgresql:postgresql:42.2.5'-->
<dependency>
<groupId>org.postgresql</groupId>
<artifactId>postgresql</artifactId>
<version>42.2.5</version>
</dependency>
<!--compile 'org.hibernate:hibernate-core:5.3.6.Final'-->
<dependency>
<groupId>org.hibernate</groupId>
<artifactId>hibernate-core</artifactId>
<version>5.3.6.Final</version>
</dependency>
<!--compile 'com.google.cloud.bigtable:bigtable-hbase-1.x:1.5.0'-->
<dependency>
<groupId>com.google.cloud.bigtable</groupId>
<artifactId>bigtable-hbase-2.x-shaded</artifactId>
<version>1.5.0</version>
</dependency>
<!--compile 'com.google.cloud:google-cloud-bigquery:1.48.0'-->
<dependency>
<groupId>com.google.cloud</groupId>
<artifactId>google-cloud-bigquery</artifactId>
<version>1.48.0</version>
</dependency>
<dependency>
<groupId>com.google.cloud</groupId>
<artifactId>google-cloud-nio</artifactId>
<version>0.83.0-alpha</version>
</dependency>
<dependency>
<groupId>org.apache.flink</groupId>
<artifactId>flink-clients_2.11</artifactId>
<version>1.5.5</version>
</dependency>

<!-- Jackson due to jinjava dependency problems -->
Copy link
Member Author

Choose a reason for hiding this comment

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

This suggests these were declared here only for the sake of jinjava removed below, so with jinjava removed I tried removing these too and nothing broke. The Jackson versions are set in <dependencyManagement> so if they're brought in transitively and the versions are important, that should take care of it already.

Copy link
Member

Choose a reason for hiding this comment

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

@davidheryanto @zhilingc do you have context on these?

I feel like we can just go ahead and remove these.

Copy link
Member Author

Choose a reason for hiding this comment

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

For a bit of extra confidence, I've run mvn dependency:analyze -DignoreNonCompile and indeed Jackson is not used in core even transitively, nor are Jinjava or Mustache. The dependency plugin's analyzer isn't perfect but it's pretty good outside of annotations.

Pushing another commit that removes more of the reported unused deps with no apparent breakage.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, the jinjava library was used as a templating engine for our SQL query in Core to retrieve batch features. But in 0.3, batch feature retrieval responsibility is moved to Serving, so we don't need need jinjava anymore in Core

<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-databind</artifactId>
<version>2.9.9.1</version>
</dependency>
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-core</artifactId>
<version>2.9.9</version>
</dependency>
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-annotations</artifactId>
<version>2.9.9</version>
</dependency>
<dependency>
<groupId>com.fasterxml.jackson.dataformat</groupId>
<artifactId>jackson-dataformat-yaml</artifactId>
<version>2.9.9</version>
</dependency>

<!--compile 'com.github.spullara.mustache.java:compiler:0.9.5'-->
<dependency>
<groupId>com.github.spullara.mustache.java</groupId>
<artifactId>compiler</artifactId>
<version>0.9.5</version>
</dependency>
<dependency>
<groupId>com.hubspot.jinjava</groupId>
Copy link
Member Author

Choose a reason for hiding this comment

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

As far as I could tell, this was unused. Unless it was here to override a version coming in transitively. If anyone knows otherwise, let me know. Nothing fails from its removal, but if it was fixing something transitive then it might need integration test coverage to surface a runtime issue.

Actually now that I look at it, the Mustache dep above might be unused too. Were these template languages used for some functionality that's been removed?

Copy link
Member

Choose a reason for hiding this comment

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

They could have been used previously for the UI. That has been cut out for now. I think it's fine to just delete these for now. If something breaks we can always add them back

<artifactId>jinjava</artifactId>
<version>2.4.12</version>
<!--exclude jackson-->
<exclusions>
<exclusion>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-databind</artifactId>
</exclusion>
<exclusion>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-core</artifactId>
</exclusion>
<exclusion>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-annotations</artifactId>
</exclusion>
</exclusions>
</dependency>
<!--compile 'io.micrometer:micrometer-core:1.0.7'-->
<dependency>
<groupId>io.micrometer</groupId>
<artifactId>micrometer-core</artifactId>
<version>1.0.7</version>
</dependency>
<!--compile 'io.micrometer:micrometer-registry-prometheus:1.0.7'-->
<dependency>
<groupId>io.micrometer</groupId>
<artifactId>micrometer-registry-statsd</artifactId>
<version>1.0.7</version>
</dependency>
<dependency>
<groupId>com.datadoghq</groupId>
<artifactId>java-dogstatsd-client</artifactId>
<version>2.6.1</version>
</dependency>

<dependency>
<groupId>org.apache.kafka</groupId>
<artifactId>kafka-clients</artifactId>
<version>2.3.0</version>
</dependency>

<!-- https://mvnrepository.com/artifact/org.apache.commons/commons-exec -->
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-exec</artifactId>
<version>1.3</version>
</dependency>

<!--compileOnly 'org.projectlombok:lombok:1.18.2'-->
<dependency>
<groupId>org.projectlombok</groupId>
<artifactId>lombok</artifactId>
<version>1.18.2</version>
<scope>provided</scope>
</dependency>

<!--testCompile "io.grpc:grpc-testing:${grpcVersion}"-->
<dependency>
<groupId>io.grpc</groupId>
<artifactId>grpc-testing</artifactId>
<version>${grpcVersion}</version>
<scope>test</scope>
</dependency>
<!--testCompile 'org.hamcrest:hamcrest-all:1.3'-->
<dependency>
<groupId>org.hamcrest</groupId>
<artifactId>hamcrest-all</artifactId>
<version>1.3</version>
<scope>test</scope>
</dependency>

<!--testCompile 'com.jayway.jsonpath:json-path-assert:2.2.0'-->
Expand All @@ -362,36 +200,14 @@
<version>2.23.0</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.squareup.okhttp3</groupId>
<artifactId>mockwebserver</artifactId>
<version>3.11.0</version>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.springframework</groupId>
<artifactId>spring-test</artifactId>
<version>5.0.8.RELEASE</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-test</artifactId>
<version>${springBootVersion}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-test-autoconfigure</artifactId>
<version>${springBootVersion}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.h2database</groupId>
<artifactId>h2</artifactId>
<version>1.4.198</version>
<scope>test</scope>
</dependency>
</dependencies>
</project>
13 changes: 5 additions & 8 deletions core/src/main/java/feast/core/job/dataflow/DataflowJobManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@

package feast.core.job.dataflow;

import static com.google.common.base.Preconditions.checkNotNull;

import com.google.api.services.dataflow.Dataflow;
import com.google.api.services.dataflow.model.Job;
import com.google.common.base.Strings;
Expand All @@ -35,7 +33,7 @@
import feast.core.model.JobInfo;
import feast.core.util.TypeConversion;
import feast.ingestion.ImportJob;
import feast.ingestion.options.ImportJobPipelineOptions;
import feast.ingestion.options.ImportOptions;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
Expand Down Expand Up @@ -128,7 +126,7 @@ public void abortJob(String dataflowJobId) {

private String submitDataflowJob(String jobName, List<FeatureSetSpec> featureSets, Store sink, boolean update) {
try {
ImportJobPipelineOptions pipelineOptions = getPipelineOptions(jobName, featureSets, sink, update);
ImportOptions pipelineOptions = getPipelineOptions(jobName, featureSets, sink, update);
DataflowPipelineJob pipelineResult = runPipeline(pipelineOptions);
String jobId = waitForJobToRun(pipelineResult);
return jobId;
Expand All @@ -138,11 +136,10 @@ private String submitDataflowJob(String jobName, List<FeatureSetSpec> featureSet
}
}

private ImportJobPipelineOptions getPipelineOptions(String jobName, List<FeatureSetSpec> featureSets, Store sink,
private ImportOptions getPipelineOptions(String jobName, List<FeatureSetSpec> featureSets, Store sink,
boolean update) throws InvalidProtocolBufferException {
String[] args = TypeConversion.convertMapToArgs(defaultOptions);
ImportJobPipelineOptions pipelineOptions = PipelineOptionsFactory.fromArgs(args)
.as(ImportJobPipelineOptions.class);
ImportOptions pipelineOptions = PipelineOptionsFactory.fromArgs(args).as(ImportOptions.class);
Printer printer = JsonFormat.printer();
List<String> featureSetsJson = new ArrayList<>();
for (FeatureSetSpec featureSet : featureSets) {
Expand All @@ -164,7 +161,7 @@ private ImportJobPipelineOptions getPipelineOptions(String jobName, List<Feature
return pipelineOptions;
}

public DataflowPipelineJob runPipeline(ImportJobPipelineOptions pipelineOptions)
public DataflowPipelineJob runPipeline(ImportOptions pipelineOptions)
throws IOException {
return (DataflowPipelineJob) ImportJob
.runPipeline(pipelineOptions);
Expand Down
Loading