You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "HiuKwok (via GitHub)" <gi...@apache.org> on 2024/02/21 06:32:43 UTC

[PR] [SPARK-46938][BUILD][CORE] Remove javax-servlet-api exclusion rule for SBT [spark]

HiuKwok opened a new pull request, #45194:
URL: https://github.com/apache/spark/pull/45194

   
   ### What changes were proposed in this pull request?
   * Update SBT build file to remove the exclusion rule for `javax-servlet-api` package, this will cause test failure, in the situation that Hive lib still rely on the old `javax` namespace servlet implementation, while Spark's servlet related code will be changed to `Jakrata` namespace.  (https://github.com/apache/spark/pull/45154)
   
   
   ### Why are the changes needed?
   * To prevent compile / test failure when Javax-servlet-api package is required under https://github.com/apache/spark/pull/45154, to provide backward compatibility during the Jetty upgrade process.
   
   ### Does this PR introduce _any_ user-facing change?
   No
   
   ### How was this patch tested?
   CI build
   
   ### Was this patch authored or co-authored using generative AI tooling?
   No


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46938][BUILD] Remove `javax-servlet-api` exclusion rule for SBT [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #45194:
URL: https://github.com/apache/spark/pull/45194#issuecomment-1956944285

   Thank you, @HiuKwok and @pan3793 . 
   Merged to master.


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46938][BUILD][CORE] Remove javax-servlet-api exclusion rule for SBT [spark]

Posted by "HiuKwok (via GitHub)" <gi...@apache.org>.
HiuKwok commented on PR #45194:
URL: https://github.com/apache/spark/pull/45194#issuecomment-1955989167

   Done.


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46938][BUILD] Remove `javax-servlet-api` exclusion rule for SBT [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #45194:
URL: https://github.com/apache/spark/pull/45194#discussion_r1497102903


##########
project/SparkBuild.scala:
##########
@@ -1075,7 +1075,6 @@ object ExcludedDependencies {
     // purpose only. Here we exclude them from the whole project scope and add them w/ yarn only.
     excludeDependencies ++= Seq(
       ExclusionRule(organization = "com.sun.jersey"),
-      ExclusionRule("javax.servlet", "javax.servlet-api"),

Review Comment:
   Ya, this is a spin-off from the original PR. We know that this is a transient status, @pan3793 . Do you have any other concern?



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46938][BUILD][CORE] Remove javax-servlet-api exclusion rule for SBT [spark]

Posted by "HiuKwok (via GitHub)" <gi...@apache.org>.
HiuKwok commented on PR #45194:
URL: https://github.com/apache/spark/pull/45194#issuecomment-1955986697

   > Oh, this PR is unable to pass CI, @HiuKwok ?
   > 
   > > this will cause test failure,
   
   It will only cause failure when Jetty upgrades to 11 AND the exclusion rule is present.
   By simply removing the rule won't fail the build I belive. 


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46938][BUILD] Remove `javax-servlet-api` exclusion rule for SBT [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #45194:
URL: https://github.com/apache/spark/pull/45194#issuecomment-1956949619

   Oh, @HiuKwok . You should not share the same JIRA ID in a spin-off PR.


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46938][BUILD] Remove `javax-servlet-api` exclusion rule for SBT [spark]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #45194:
URL: https://github.com/apache/spark/pull/45194#discussion_r1497094373


##########
project/SparkBuild.scala:
##########
@@ -1075,7 +1075,6 @@ object ExcludedDependencies {
     // purpose only. Here we exclude them from the whole project scope and add them w/ yarn only.
     excludeDependencies ++= Seq(
       ExclusionRule(organization = "com.sun.jersey"),
-      ExclusionRule("javax.servlet", "javax.servlet-api"),

Review Comment:
   this is intended, to avoid conflict with `jakarta.servlet-api:4.x`, the tricky thing is that `jakarta.servlet-api:4.x` actually contains `javax.servlet` classes



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46938][BUILD][CORE] Remove javax-servlet-api exclusion rule for SBT [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #45194:
URL: https://github.com/apache/spark/pull/45194#issuecomment-1955987952

   Could you revise the PR description? You don't need to mention any failure here. It only causes confusions.


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46938][BUILD] Remove `javax-servlet-api` exclusion rule for SBT [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun closed pull request #45194: [SPARK-46938][BUILD] Remove `javax-servlet-api` exclusion rule for SBT
URL: https://github.com/apache/spark/pull/45194


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46938][BUILD][CORE] Remove javax-servlet-api exclusion rule for SBT [spark]

Posted by "HiuKwok (via GitHub)" <gi...@apache.org>.
HiuKwok commented on PR #45194:
URL: https://github.com/apache/spark/pull/45194#issuecomment-1955987490

   @dongjoon-hyun Your understanding is correct 👍 


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46938][BUILD] Remove `javax-servlet-api` exclusion rule for SBT [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #45194:
URL: https://github.com/apache/spark/pull/45194#discussion_r1497102903


##########
project/SparkBuild.scala:
##########
@@ -1075,7 +1075,6 @@ object ExcludedDependencies {
     // purpose only. Here we exclude them from the whole project scope and add them w/ yarn only.
     excludeDependencies ++= Seq(
       ExclusionRule(organization = "com.sun.jersey"),
-      ExclusionRule("javax.servlet", "javax.servlet-api"),

Review Comment:
   Ya, this is a spin-off from the following original PR. We know that this is a transient status, @pan3793 . Do you have any other concern?
   - #45154



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46938][BUILD] Remove `javax-servlet-api` exclusion rule for SBT [spark]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #45194:
URL: https://github.com/apache/spark/pull/45194#discussion_r1497152516


##########
project/SparkBuild.scala:
##########
@@ -1075,7 +1075,6 @@ object ExcludedDependencies {
     // purpose only. Here we exclude them from the whole project scope and add them w/ yarn only.
     excludeDependencies ++= Seq(
       ExclusionRule(organization = "com.sun.jersey"),
-      ExclusionRule("javax.servlet", "javax.servlet-api"),

Review Comment:
   I get the point. The change makes sense to me.



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org