[SPARK-47118][BUILD][CORE][SQL][UI] Migrate from Jetty 10 to Jetty 11 #45154
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
SQL
STRUCTURED STREAMING
KUBERNETES
WEB UI
BUILD
YARN
CORE
DSTREAM
CONNECT
labels
|
Thank you for making this effort, @HiuKwok . Could you resolve the conflicts? |
HiuKwok
force-pushed
the
ft-hf-SPARK-45522-jetty-11
branch
from
67a4712
to
c243f22
Compare
|
@dongjoon-hyun done, awaiting for the CI build. |
|
Checking on the failed tests |
|
The MR is ready for review. |
|
Thank you, @HiuKwok ! |
| "-Wconf:cat=deprecation&msg=it will become a keyword in Scala 3:e" | ||
| "-Wconf:cat=deprecation&msg=it will become a keyword in Scala 3:e", | ||
| // SPARK-46938 to prevent enum scan on pmml-model, under spark-mllib module. | ||
| "-Wconf:cat=other&site=org.dmg.pmml.*:w" |
There was a problem hiding this comment.
Do we need this? It seems that this PR only adds the following.
<dependency>
<groupId>javax.xml.bind</groupId>
<artifactId>jaxb-api</artifactId>
</dependency>
There was a problem hiding this comment.
If this is the result of the following removal, can we proceed this in a separate PR first, @HiuKwok ?
- ExclusionRule("javax.servlet", "javax.servlet-api"),
There was a problem hiding this comment.
1. jaxb-api
I will need to double-check whatever is caused by the jaxb-api or not and I will get back to you.
FYI, this is the error I faced earlier if that helps.
[error] While parsing annotations in /home/runner/.cache/coursier/v1/https/maven-central.storage-download.googleapis.com/maven2/org/jpmml/pmml-model/1.4.8/pmml-model-1.4.8.jar(org/dmg/pmml/regression/RegressionModel.class), could not find FIELD in enum <none>.
[error] This is likely due to an implementation restriction: an annotation argument cannot refer to a member of the annotated class (scala/bug#7014).
[error] Applicable -Wconf / @nowarn filters for this fatal warning: msg=<part of the message>, cat=other, site=org.dmg.pmml.regression.RegressionModel
[error] While parsing annotations in /home/runner/.cache/coursier/v1/https/maven-central.storage-download.googleapis.com/maven2/org/jpmml/pmml-model/1.4.8/pmml-model-1.4.8.jar(org/dmg/pmml/DataField.class), could not find FIELD in enum <none>.
[error] This is likely due to an implementation restriction: an annotation argument cannot refer to a member of the annotated class (scala/bug#7014).
[error] Applicable -Wconf / @nowarn filters for this fatal warning: msg=<part of the message>, cat=other, site=org.dmg.pmml.DataField
[error] While parsing annotations in /home/runner/.cache/coursier/v1/https/maven-central.storage-download.googleapis.com/maven2/org/jpmml/pmml-model/1.4.8/pmml-model-1.4.8.jar(org/dmg/pmml/regression/RegressionTable.class), could not find FIELD in enum <none>.
[error] This is likely due to an implementation restriction: an annotation argument cannot refer to a member of the annotated class (scala/bug#7014).
[error] Applicable -Wconf / @nowarn filters for this fatal warning: msg=<part of the message>, cat=other, site=org.dmg.pmml.regression.RegressionTable
[error] While parsing annotations in /home/runner/.cache/coursier/v1/https/maven-central.storage-download.googleapis.com/maven2/org/jpmml/pmml-model/1.4.8/pmml-model-1.4.8.jar(org/dmg/pmml/regression/NumericPredictor.class), could not find FIELD in enum <none>.
and reference from one of my historical build.
https://github.com/HiuKwok/spark/actions/runs/7932736253/job/21659910112
2. ExclusionRule
Yes, I Will create a separate PR for it.
...ve-thriftserver/src/main/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java
Outdated
Show resolved
Hide resolved
|
BTW, I realized that we need to make this PR less intrusive by reducing the surface of change. We had better introduce a layer inside Spark. Not only for this but also for the migration from Jetty 11 to Jerry 12.
I made a PR for that. |
could you please elaborate more on the invocation chains? e.g. provide stacktrace |
Sure, I review the existing code change and check whether how can we centralise the servlet reference, mostly likely this will be another MR prior to this upgrade. |
This is the stack trace I had from one of the old builds during the development, which asks for Also here is the old build which threw the exception, if this provides more clarity on the issue. |
|
@dongjoon-hyun |
mridulm
reviewed
Feb 21, 2024
|
@HiuKwok Hmm... seems a class initialization would trigger all referenced classes loading. we may need to fix it on Hive side by breaking the |
|
Ya, I had the same question initially. For now, I have no idea about that, @mridulm . Definitely, Apache Spark didn't handle them properly in both
|
Co-authored-by: Mridul Muralidharan <1591700+mridulm@users.noreply.github.com>
|
Thank you for update, @HiuKwok . |
Indeed that is a maven |
|
Thank you. I'll revisit this PR tomorrow morning with fresh eyes, @HiuKwok . I'm currently in California (PST) timezone. |
Sure, take your time 👍 |
Sure, I will open a separate JIRA under SPARK-47046 for that. |
dongjoon-hyun
pushed a commit
that referenced
this pull request
Feb 26, 2024
….servlet-api` dependency scope in `connect/server` module ### What changes were proposed in this pull request? This is a follow up change from #45154, to remove redundant `<scope>` for both servlet-api as `compile` is the default scope. ### Why are the changes needed? ### 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 Closes #45258 from HiuKwok/ft-hf-jetty-deps-scope. Authored-by: HiuFung Kwok <hiufkwok@gmail.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
a0x8o
added a commit
to a0x8o/spark
that referenced
this pull request
Feb 26, 2024
….servlet-api` dependency scope in `connect/server` module ### What changes were proposed in this pull request? This is a follow up change from apache/spark#45154, to remove redundant `<scope>` for both servlet-api as `compile` is the default scope. ### Why are the changes needed? ### 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 Closes #45258 from HiuKwok/ft-hf-jetty-deps-scope. Authored-by: HiuFung Kwok <hiufkwok@gmail.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
TakawaAkirayo
pushed a commit
to TakawaAkirayo/spark
that referenced
this pull request
Mar 4, 2024
….servlet-api` dependency scope in `connect/server` module ### What changes were proposed in this pull request? This is a follow up change from apache#45154, to remove redundant `<scope>` for both servlet-api as `compile` is the default scope. ### Why are the changes needed? ### 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 Closes apache#45258 from HiuKwok/ft-hf-jetty-deps-scope. Authored-by: HiuFung Kwok <hiufkwok@gmail.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
ericm-db
pushed a commit
to ericm-db/spark
that referenced
this pull request
Mar 5, 2024
### What changes were proposed in this pull request? This is an upgrade ticket to bump the Jetty version from 10 to 11, and the Jersey version from 2 to 3, so the project can gradually move away from Javax.servlet package and adopt the Jakrata standard, code changes on a high level involves: 1. Bump `jakarta.servlet-api` from 4 to 5 in order to adopt the new namespace. 2. Re-introduce the `javax.servlet-api` and `jaxb-api` jars, as Hive-related Jars are using old servelet reference internally, or else it will throw classNotFound during test and runtime, also this makes us decouple the HIve upgrade from this MR. 3. Rewrite the Tservlet class from LIbThrift with alternative namespace `Jakrata` instead of `Javax`, again this helps us to decouple the LIbThrift upgrade task from this MR, or else we will need to wait for Hive 4. 4. Update `MimaExclude.scala` to exclude the breaking compatibility that introduced by javax -> jakrata. 5. Update Scalac option to prevent the enum scan on third-party jar `org.dmg.pmml` and its failed the build. 6. Update ALL internal servlet implementation from Javax to Jakrata. ### Why are the changes needed? To the Spark one step closer to the latest version of Jetty and Jersey, to receive up-to-date security fixes. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? CI build & Unit test ### Was this patch authored or co-authored using generative AI tooling? No Closes apache#45154 from HiuKwok/ft-hf-SPARK-45522-jetty-11. Lead-authored-by: HiuFung Kwok <hiufkwok@gmail.com> Co-authored-by: HiuFung Kwok <37996731+HiuKwok@users.noreply.github.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
ericm-db
pushed a commit
to ericm-db/spark
that referenced
this pull request
Mar 5, 2024
….servlet-api` dependency scope in `connect/server` module ### What changes were proposed in this pull request? This is a follow up change from apache#45154, to remove redundant `<scope>` for both servlet-api as `compile` is the default scope. ### Why are the changes needed? ### 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 Closes apache#45258 from HiuKwok/ft-hf-jetty-deps-scope. Authored-by: HiuFung Kwok <hiufkwok@gmail.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
3 tasks
|
@HiuKwok @dongjoon-hyun I meet an issue on the latest master branch that seems related to this change. The brief reproducible steps are:
|
|
also cc the 4.0.0-preview1 release manager @cloud-fan |
|
Please file a proper JIRA issue, @pan3793 . |
|
I raised SPARK-48238. And I have no solution without reverting javax => jakarta namespace migration yet. |
|
According to https://issues.apache.org/jira/browse/SPARK-48238 , we need to revert this. I'll cut another RC after it. |
|
Hi @HiuKwok, I noticed this PR migrated |
Actually not just for javax.servlet|wr.rs, but we should also review all usages of Javax packages. |
dongjoon-hyun
pushed a commit
that referenced
this pull request
Oct 15, 2024
### What changes were proposed in this pull request? - To Remove the dependency of `javax.ws.rs.ws-rs-api` as it's no longer required. Prior discussion can be found on: - #41340 - #45154 ### Why are the changes needed? In the past, the codebase used to have a few .scala classes referencing and using the `ws-rs-api`, such as b7fdc23#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R624-R627 However as the time passed by, all usages of `ws-rs-api` are either got removed / refactored. Hence there is no need to have it import on root POM as now and we can always re-introduce it later, if the usage can be justified again. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Unit-test, to make sure the codebase is not impacted by the removal of the dependency. ### Was this patch authored or co-authored using generative AI tooling? No Closes #48461 from hiufung-kwok/ft-hf-SPARK-49963-remove-ws-rs-api. Authored-by: HiuFung Kwok <hiufung.kwok.852@gmail.com> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
himadripal
pushed a commit
to himadripal/spark
that referenced
this pull request
Oct 19, 2024
### What changes were proposed in this pull request? - To Remove the dependency of `javax.ws.rs.ws-rs-api` as it's no longer required. Prior discussion can be found on: - apache#41340 - apache#45154 ### Why are the changes needed? In the past, the codebase used to have a few .scala classes referencing and using the `ws-rs-api`, such as apache@b7fdc23#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R624-R627 However as the time passed by, all usages of `ws-rs-api` are either got removed / refactored. Hence there is no need to have it import on root POM as now and we can always re-introduce it later, if the usage can be justified again. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Unit-test, to make sure the codebase is not impacted by the removal of the dependency. ### Was this patch authored or co-authored using generative AI tooling? No Closes apache#48461 from hiufung-kwok/ft-hf-SPARK-49963-remove-ws-rs-api. Authored-by: HiuFung Kwok <hiufung.kwok.852@gmail.com> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
This is an upgrade ticket to bump the Jetty version from 10 to 11, and the Jersey version from 2 to 3, so the project can gradually move away from Javax.servlet package and adopt the Jakrata standard, code changes on a high level involves:
Bump
jakarta.servlet-apifrom 4 to 5 in order to adopt the new namespace.Re-introduce the
javax.servlet-apiandjaxb-apijars, as Hive-related Jars are using old servelet reference internally, or else it will throw classNotFound during test and runtime, also this makes us decouple the HIve upgrade from this MR.Rewrite the Tservlet class from LIbThrift with alternative namespace
Jakratainstead ofJavax, again this helps us to decouple the LIbThrift upgrade task from this MR, or else we will need to wait for Hive 4.Update
MimaExclude.scalato exclude the breaking compatibility that introduced by javax -> jakrata.Update Scalac option to prevent the enum scan on third-party jar
org.dmg.pmmland its failed the build.Update ALL internal servlet implementation from Javax to Jakrata.
Why are the changes needed?
To the Spark one step closer to the latest version of Jetty and Jersey, to receive up-to-date security fixes.
Does this PR introduce any user-facing change?
No
How was this patch tested?
CI build & Unit test
Was this patch authored or co-authored using generative AI tooling?
No