You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by GitBox <gi...@apache.org> on 2021/05/25 04:47:58 UTC
[GitHub] [drill] laurentgo opened a new pull request #2236: DRILL-7135: Update to Jetty 9.4
laurentgo opened a new pull request #2236:
URL: https://github.com/apache/drill/pull/2236
# [DRILL-7135](https://issues.apache.org/jira/browse/DRILL-7135): Update to Jetty 9.4
## Description
Jetty 9.3 is not a supported version anymore and doesn't get any
security fixes.
Update Jetty version used by Drill to the latest 9.4 version (9.4.41).
## Documentation
N/A
## Testing
N/A
--
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:
users@infra.apache.org
[GitHub] [drill] paul-rogers commented on a change in pull request #2236: DRILL-7135: Update to Jetty 9.4
Posted by GitBox <gi...@apache.org>.
paul-rogers commented on a change in pull request #2236:
URL: https://github.com/apache/drill/pull/2236#discussion_r641005621
##########
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:
Thanks for the explanation. Not much we can do, I suppose.
--
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:
users@infra.apache.org
[GitHub] [drill] luocooong merged pull request #2236: DRILL-7135: Update to Jetty 9.4
Posted by GitBox <gi...@apache.org>.
luocooong merged pull request #2236:
URL: https://github.com/apache/drill/pull/2236
--
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:
users@infra.apache.org
[GitHub] [drill] laurentgo commented on a change in pull request #2236: DRILL-7135: Update to Jetty 9.4
Posted by GitBox <gi...@apache.org>.
laurentgo commented on a change in pull request #2236:
URL: https://github.com/apache/drill/pull/2236#discussion_r640114277
##########
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:
Unfortunately the mini dfs cluster tests failed without this dependency, which is why this a test dependency and not a compile/runtime one luckily...
--
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:
users@infra.apache.org
[GitHub] [drill] laurentgo commented on a change in pull request #2236: DRILL-7135: Update to Jetty 9.4
Posted by GitBox <gi...@apache.org>.
laurentgo commented on a change in pull request #2236:
URL: https://github.com/apache/drill/pull/2236#discussion_r640162551
##########
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:
There are more simplifications like this for Jersey and Jackson
--
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:
users@infra.apache.org
[GitHub] [drill] laurentgo commented on pull request #2236: DRILL-7135: Update to Jetty 9.4
Posted by GitBox <gi...@apache.org>.
laurentgo commented on pull request #2236:
URL: https://github.com/apache/drill/pull/2236#issuecomment-849709214
@paul-rogers any objection to get this change merged?
--
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:
users@infra.apache.org
[GitHub] [drill] paul-rogers commented on a change in pull request #2236: DRILL-7135: Update to Jetty 9.4
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org