paul-rogers commented on a change in pull request #2236:
URL: https://github.com/apache/drill/pull/2236#discussion_r640055304
##########
File path: exec/java-exec/pom.xml
##########
@@ -502,6 +502,11 @@
</exclusion>
</exclusions>
</dependency>
+ <dependency>
+ <groupId>org.apache.hadoop</groupId>
+ <artifactId>hadoop-yarn-api</artifactId>
+ <scope>test</scope>
+ </dependency>
Review comment:
Is this needed here? Drill-on-YARN uses the YARN API. We did try hard to
avoid adding a YARN dependency to Drill itself, since Drill can run
stand-alone, under YARN, under K8s, etc.
##########
File path: pom.xml
##########
@@ -2850,43 +2850,10 @@
<!--Eclipse Jetty dependecies-->
<dependency>
<groupId>org.eclipse.jetty</groupId>
- <artifactId>jetty-server</artifactId>
- <version>${jetty.version}</version>
- </dependency>
- <dependency>
- <groupId>org.eclipse.jetty</groupId>
- <artifactId>jetty-servlet</artifactId>
- <version>${jetty.version}</version>
- </dependency>
- <dependency>
- <groupId>org.eclipse.jetty</groupId>
- <artifactId>jetty-servlets</artifactId>
- <version>${jetty.version}</version>
- </dependency>
- <dependency>
- <groupId>org.eclipse.jetty</groupId>
- <artifactId>jetty-security</artifactId>
- <version>${jetty.version}</version>
- </dependency>
- <dependency>
- <groupId>org.eclipse.jetty</groupId>
- <artifactId>jetty-util</artifactId>
- <version>${jetty.version}</version>
- </dependency>
- <dependency>
- <groupId>org.eclipse.jetty</groupId>
- <artifactId>jetty-io</artifactId>
- <version>${jetty.version}</version>
- </dependency>
- <dependency>
- <groupId>org.eclipse.jetty</groupId>
- <artifactId>jetty-webapp</artifactId>
- <version>${jetty.version}</version>
- </dependency>
- <dependency>
- <groupId>org.eclipse.jetty</groupId>
- <artifactId>jetty-xml</artifactId>
+ <artifactId>jetty-bom</artifactId>
<version>${jetty.version}</version>
+ <type>pom</type>
+ <scope>import</scope>
Review comment:
Nice simplification!
##########
File path:
drill-yarn/src/main/java/org/apache/drill/yarn/appMaster/http/WebServer.java
##########
@@ -306,18 +304,18 @@ private ConstraintSecurityHandler createSecurityHandler()
{
}
/**
- * It creates A {@link SessionHandler} which contains a {@link
HashSessionManager}
+ * It creates A {@link SessionHandler}
Review comment:
Sorry to make you do this twice. The DoY web server was pretty much a
copy/paste clone of the Drill exec version. The goal was to factor out a common
set of code, but that never quite happened...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
