[GitHub] [drill] vdiravka commented on a change in pull request #1681: DRILL-7051: Upgrade jetty
vdiravka commented on a change in pull request #1681: DRILL-7051: Upgrade jetty URL: https://github.com/apache/drill/pull/1681#discussion_r268551237 ## File path: pom.xml ## @@ -85,6 +85,7 @@ 0.9.10 1.8.2 4.0.2 +9.4.15.v20190215 Review comment: Finally jetty 9.3 is chosen for Drill. Jetty dependencies are used in `java-exec pom.xml`, but I've left versions control in `dependencyManagement` block of root POM to avoid picking invalid jetty version by maven, in case when some libs will have other version. For instance we can't exclude jetty from `hadoop-common` and `hbase` dependencies. But they have different jetty minor versions: [9.3.24.v20180605](https://github.com/apache/hadoop/blob/trunk/hadoop-project/pom.xml#L38) for Hadoop [9.3.19.v20170502](https://github.com/apache/hbase/blob/rel/2.1.0/pom.xml#L1352) for HBase 2.1 and [9.3.25.v20180904](https://github.com/apache/hbase/blob/master/pom.xml#L1529) for master HBase version. I didn't find any API compatibility differences between these jetty minor versions (only 9.4 has it). Possibly in future we can consider shade Jetty version in Drill, [DRILL-7135](https://issues.apache.org/jira/browse/DRILL-7135). Not sure that is necessary for now. 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 With regards, Apache Git Services
[GitHub] [drill] vdiravka commented on a change in pull request #1681: DRILL-7051: Upgrade jetty
vdiravka commented on a change in pull request #1681: DRILL-7051: Upgrade jetty URL: https://github.com/apache/drill/pull/1681#discussion_r265904521 ## File path: pom.xml ## @@ -85,6 +85,7 @@ 0.9.10 1.8.2 4.0.2 +9.4.15.v20190215 Review comment: It looks like HBase was earlier on Jetty 9.4 version, but there is an issue with `MiniDFSCluster` [HBASE-18943](https://issues.apache.org/jira/browse/HBASE-18943), [HBASE-19390](https://issues.apache.org/jira/browse/HBASE-19390), so they reverted to 9.3 Jetty version. It was the same reason why I have updated Jetty in the scope of [DRILL-6540: Upgrade to HADOOP-3.1 libraries](https://issues.apache.org/jira/browse/DRILL-6540). It also appears that new Jersey version requires Jetty 9.4 version, see [comment](https://github.com/apache/hbase/blob/master/pom.xml#L1533) in HBase. Possibly we should consider the newest Jetty and Jersey versions only after [HBASE-19256](https://issues.apache.org/jira/browse/HBASE-19256) resolution. So I will check my changes for Hadoop3.1 libs version in Drill with newest Jetty and Jersey. If there are some issues with them, I will put to Drill the same Jetty and Jersey versions as in HBase project. 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 With regards, Apache Git Services
[GitHub] [drill] vdiravka commented on a change in pull request #1681: DRILL-7051: Upgrade jetty
vdiravka commented on a change in pull request #1681: DRILL-7051: Upgrade jetty URL: https://github.com/apache/drill/pull/1681#discussion_r264911104 ## File path: pom.xml ## @@ -85,6 +85,7 @@ 0.9.10 1.8.2 4.0.2 +9.4.15.v20190215 Review comment: I will investigate how hbase and mapr-db jetty dependencies affect Drill. 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 With regards, Apache Git Services
[GitHub] [drill] vdiravka commented on a change in pull request #1681: DRILL-7051: Upgrade jetty
vdiravka commented on a change in pull request #1681: DRILL-7051: Upgrade jetty URL: https://github.com/apache/drill/pull/1681#discussion_r263127462 ## File path: pom.xml ## @@ -85,6 +85,7 @@ 0.9.10 1.8.2 4.0.2 +9.4.15.v20190215 Review comment: In current PR I have added jetty-dependencies to the `dependencyManagement` block only. It was done in scope of DRILL-68.. I have added a [comment](https://issues.apache.org/jira/browse/DRILL-6540?focusedCommentId=16606306&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16606306) that it was necessary for the HBase unit tests. So it can't be excluded. Also I checked `mvn dependency:tree -Dincludes=org.eclipse.jetty` without `dependencyManagement` block and found different versions of Jetty ``` [INFO] org.apache.drill.contrib:drill-format-mapr:jar:1.16.0-SNAPSHOT [INFO] +- com.mapr.fs:mapr-hbase:jar:6.1.0-mapr:compile [INFO] | \- org.apache.hbase:hbase-server:jar:2.1.1:compile [INFO] | +- org.apache.hbase:hbase-http:jar:2.1.1:compile [INFO] | | +- org.eclipse.jetty:jetty-util:jar:9.3.19.v20170502:compile [INFO] | | \- org.eclipse.jetty:jetty-util-ajax:jar:9.3.19.v20170502:compile [INFO] | \- org.eclipse.jetty:jetty-webapp:jar:9.3.19.v20170502:compile [INFO] |\- org.eclipse.jetty:jetty-xml:jar:9.3.19.v20170502:compile [INFO] \- org.apache.drill.exec:drill-java-exec:jar:1.16.0-SNAPSHOT:compile [INFO]+- org.eclipse.jetty:jetty-server:jar:9.4.15.v20190215:compile [INFO]| +- org.eclipse.jetty:jetty-http:jar:9.4.15.v20190215:compile [INFO]| \- org.eclipse.jetty:jetty-io:jar:9.4.15.v20190215:compile [INFO]+- org.eclipse.jetty:jetty-servlet:jar:9.4.15.v20190215:compile [INFO]| \- org.eclipse.jetty:jetty-security:jar:9.4.15.v20190215:compile [INFO]+- org.eclipse.jetty:jetty-servlets:jar:9.4.15.v20190215:compile [INFO]\- org.glassfish.jersey.containers:jersey-container-jetty-servlet:jar:2.8:compile [INFO] \- org.glassfish.jersey.containers:jersey-container-jetty-http:jar:2.8:compile [INFO] \- org.eclipse.jetty:jetty-continuation:jar:9.1.1.v20140108:compile [INFO] ``` Therefore since Jetty dependencies are used not only in `java-exec` it is nesessary to handle their versions in Drill root POM file. 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 With regards, Apache Git Services
[GitHub] [drill] vdiravka commented on a change in pull request #1681: DRILL-7051: Upgrade jetty
vdiravka commented on a change in pull request #1681: DRILL-7051: Upgrade jetty URL: https://github.com/apache/drill/pull/1681#discussion_r263128156 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/DrillSpnegoAuthenticator.java ## @@ -152,7 +153,7 @@ private Authentication authenticateSession(ServletRequest request, ServletRespon } response.setContentLength(0); -final HttpChannel channel = HttpChannel.getCurrentHttpChannel(); +final HttpChannel channel = HttpConnection.getCurrentConnection().getHttpChannel(); final Response base_response = channel.getResponse(); final Request base_request = channel.getRequest(); Review comment: Thanks, it looks better. Also I have dropped usage of `base_response` local variable. 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 With regards, Apache Git Services
