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.

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

Proposed changes to the update #42

Merged
merged 1 commit into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
142 changes: 20 additions & 122 deletions java/barrage-core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -26,21 +26,15 @@
<artifactId>barrage-core</artifactId>
<name>Deephaven Barrage Core</name>
<description>An RPC mechanism for transferring ticking Arrow data.</description>
<packaging>jar</packaging>

<properties>
<dep.grpc.version>1.65.0</dep.grpc.version>
<dep.protobuf.version>3.25.4</dep.protobuf.version>
<dep.guava-bom.version>33.3.1-jre</dep.guava-bom.version>
<forkCount>1</forkCount>
Copy link
Member Author

Choose a reason for hiding this comment

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

We don't run the tests that used this, so I removed it (and the test run that didnt run tests).

<dep.guava.version>33.3.1-jre</dep.guava.version>
<protoOutDir>${project.build.directory}/generated-sources/protobuf</protoOutDir>
Copy link
Member Author

Choose a reason for hiding this comment

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

extracted this rather than list the string three times (previously twice, but i added a new usage)

</properties>

<dependencies>
<dependency>
<groupId>io.grpc</groupId>
<artifactId>grpc-core</artifactId>
<version>${dep.grpc.version}</version>
</dependency>
<dependency>
<groupId>io.grpc</groupId>
<artifactId>grpc-protobuf</artifactId>
Expand All @@ -49,6 +43,7 @@
<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
<version>${dep.guava.version}</version>
</dependency>
<dependency>
<groupId>io.grpc</groupId>
Expand All @@ -66,28 +61,10 @@
<version>${dep.grpc.version}</version>
</dependency>
<dependency>
<groupId>org.apache.arrow</groupId>
<artifactId>flight-core</artifactId>
</dependency>

<dependency>
<groupId>com.google.api.grpc</groupId>
<artifactId>proto-google-common-protos</artifactId>
<version>1.12.0</version>
<scope>test</scope>
Copy link
Member Author

Choose a reason for hiding this comment

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

We never ran any tests with this, so I removed it.

<groupId>org.apache.arrow</groupId>
<artifactId>flight-core</artifactId>
</dependency>
</dependencies>
<dependencyManagement>
<dependencies>
<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava-bom</artifactId>
<version>${dep.guava-bom.version}</version>
<type>pom</type>
<scope>import</scope>
</dependency>
</dependencies>
</dependencyManagement>
<build>
<extensions>
<extension>
Expand All @@ -97,69 +74,6 @@
</extension>
</extensions>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-shade-plugin</artifactId>
Copy link
Member Author

Choose a reason for hiding this comment

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

We produced a shaded build and also an assembly, but I'm not clear why - with a shaded copy of protobuf? Note too that this only applied to BrowserFlight, and not Barrage itself.

<version>3.4.0</version>
<executions>
<execution>
<id>shade-main</id>
<phase>package</phase>
<goals>
<goal>shade</goal>
</goals>
<configuration>
<shadedArtifactAttached>true</shadedArtifactAttached>
<shadedClassifierName>shaded</shadedClassifierName>
<artifactSet>
<includes>
<include>io.grpc:*</include>
<include>com.google.protobuf:*</include>
</includes>
</artifactSet>
<relocations>
<relocation>
<pattern>com.google.protobuf</pattern>
<shadedPattern>io.deephaven.barrage.com.google.protobuf</shadedPattern>
</relocation>
</relocations>
<transformers>
<transformer implementation="org.apache.maven.plugins.shade.resource.ServicesResourceTransformer" />
</transformers>
</configuration>
</execution>
<execution>
<id>shade-ext</id>
<phase>package</phase>
<goals>
<goal>shade</goal>
</goals>
<configuration>
<shadedArtifactAttached>true</shadedArtifactAttached>
<shadedClassifierName>shaded-ext</shadedClassifierName>
<artifactSet>
<includes>
<include>io.grpc:*</include>
<include>com.google.protobuf:*</include>
</includes>
</artifactSet>
<relocations>
<relocation>
<pattern>com.google.protobuf</pattern>
<shadedPattern>io.deephaven.barrage.com.google.protobuf</shadedPattern>
</relocation>
<relocation>
<pattern>com.google.common</pattern>
<shadedPattern>io.deephaven.barrage.com.google.common</shadedPattern>
</relocation>
</relocations>
<transformers>
<transformer implementation="org.apache.maven.plugins.shade.resource.ServicesResourceTransformer" />
</transformers>
</configuration>
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.xolstice.maven.plugins</groupId>
<artifactId>protobuf-maven-plugin</artifactId>
Expand All @@ -175,7 +89,7 @@
<id>src</id>
<configuration>
<protoSourceRoot>${basedir}/../../format/</protoSourceRoot>
<outputDirectory>${project.build.directory}/generated-sources/protobuf</outputDirectory>
<outputDirectory>${protoOutDir}</outputDirectory>
<includes>
<!-- Note we only want to include the BrowserFlight service! -->
<include>**/BrowserFlight.proto</include>
Expand All @@ -188,22 +102,24 @@
</execution>
</executions>
</plugin>
<!-- add the license header to the generated files -->
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-dependency-plugin</artifactId>
<groupId>com.mycila</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.

previous artifacts had no headers in the proto output - not sure it matters, but now we have them.

<artifactId>license-maven-plugin</artifactId>
<version>4.6</version>
<configuration>
<header>${basedir}/../../header</header>
<includes>
<include>**/*.java</include>
</includes>
<defaultBasedir>${protoOutDir}</defaultBasedir>
</configuration>
<executions>
<execution>
<id>analyze</id>
<phase>verify</phase>
<phase>process-sources</phase>
<goals>
<goal>analyze-only</goal>
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 was elevated to the parent pom so that it runs on both format and core. Verified by removing guava and trying to build, it complained that guava was missing again.

<goal>format</goal>
</goals>
<configuration>
<ignoredDependencies>
<ignoredDependency>io.grpc:grpc-core:jar:${dep.grpc.version}</ignoredDependency>
<ignoredDependency>io.grpc:grpc-context:jar:${dep.grpc.version}</ignoredDependency>
</ignoredDependencies>
</configuration>
</execution>
</executions>
</plugin>
Expand All @@ -220,30 +136,12 @@
</goals>
<configuration>
<sources>
<source>${project.build.directory}/generated-sources/protobuf</source>
<source>${protoOutDir}</source>
</sources>
</configuration>
</execution>
</executions>
</plugin>
<plugin>
<artifactId>maven-assembly-plugin</artifactId>
<version>3.4.2</version>
<configuration>
<descriptorRefs>
<descriptorRef>jar-with-dependencies</descriptorRef>
</descriptorRefs>
</configuration>
<executions>
<execution>
<id>make-assembly</id>
<phase>package</phase>
<goals>
<goal>single</goal>
</goals>
</execution>
</executions>
</plugin>
</plugins>
</build>
</project>
7 changes: 2 additions & 5 deletions java/format/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,8 @@ cd $BARRAGE_HOME
rm -rf java/format/src
# regenerate from the .fbs files
flatc --java -o java/format/src/main/java format/*.fbs
# prepend license header
find java/format/src -type f | while read file; do
(cat header | while read line; do echo "// $line"; done; cat $file) > $file.tmp
mv $file.tmp $file
done
# generate license headers
mvn compile
```

3. Ensure any new files are added to the git repository:
Expand Down
25 changes: 24 additions & 1 deletion java/format/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
</parent>

<artifactId>barrage-format</artifactId>
<packaging>jar</packaging>
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 is the default value, so unnecessary to explicitly list

<name>Barrage Format</name>
<description>Generated Java files from the IPC Flatbuffer definitions.</description>

Expand All @@ -35,4 +34,28 @@
</dependency>
</dependencies>

<build>
<plugins>
<!-- add the license header to the committed generated files -->
<plugin>
<groupId>com.mycila</groupId>
<artifactId>license-maven-plugin</artifactId>
<version>4.6</version>
<configuration>
<header>${basedir}/../../header</header>
<includes>
<include>**/*.java</include>
</includes>
</configuration>
<executions>
<execution>
<phase>process-sources</phase>
<goals>
<goal>format</goal>
</goals>
</execution>
</executions>
</plugin>
</plugins>
</build>
</project>
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
// Copyright 2020 Deephaven Data Labs
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// automatically generated by the FlatBuffers compiler, do not modify

/*
* Copyright 2020 Deephaven Data Labs
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.deephaven.barrage.flatbuf;

@SuppressWarnings("unused")
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
// Copyright 2020 Deephaven Data Labs
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// automatically generated by the FlatBuffers compiler, do not modify

/*
* Copyright 2020 Deephaven Data Labs
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.deephaven.barrage.flatbuf;

import com.google.flatbuffers.BaseVector;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
// Copyright 2020 Deephaven Data Labs
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// automatically generated by the FlatBuffers compiler, do not modify

/*
* Copyright 2020 Deephaven Data Labs
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.deephaven.barrage.flatbuf;

import com.google.flatbuffers.BaseVector;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
// Copyright 2020 Deephaven Data Labs
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// automatically generated by the FlatBuffers compiler, do not modify

/*
* Copyright 2020 Deephaven Data Labs
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.deephaven.barrage.flatbuf;

import com.google.flatbuffers.BaseVector;
Expand Down
Loading
Loading