-
Notifications
You must be signed in to change notification settings - Fork 28.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SPARK-45522][BUILD][CORE][SQL][UI] Migrate from Jetty 9 to Jetty 10 #43765
Conversation
0df2668
to
59809f6
Compare
Checking the build failure |
[error] /home/runner/work/spark/spark/core/src/test/scala/org/apache/spark/ui/UISuite.scala:415: File line length exceeds 100 characters |
I have fixed the code style and other miscellaneous linter errors, right now I'm investigating the Python test case fail, for PySpark connect. |
This is a good step - we should try to get to Jetty 11 next before Spark 4, or even Jetty 12 |
Co-authored-by: Kent Yao <[email protected]>
Co-authored-by: YangJie <[email protected]>
Resolved all comments. |
Merged to master |
@srowen @dongjoon-hyun @LuciferYang @yaooqinn @cloud-fan @HiuKwok Unfortunately, we identified the compatibility issue on Jetty 11, and it is likely to revert Jetty 11 upgrading, see details in SPARK-48238. If we downgrade Jetty, which version we want to keep, 9 or 10? I would prefer 9, because I think
jetty/jetty.project#10103 (comment)
|
+1 for 9.x |
Agree. |
Thanks for the discussion here! @HiuKwok are you going to do the revert work? If not please let us know and we can help. |
What changes were proposed in this pull request?
This is an upgrade ticket to bump the Jetty version from 9 to 10.
This PR aims to bring incremental Jetty upgrades to Spark, as Jetty 9 support already reached EOL.
Why are the changes needed?
Jetty 9 is already beyond EOL, which means that we won't receive any security fix onward for Spark.
Does this PR introduce any user-facing change?
No, SNI host check is now defaulted to true on embedded Jetty, hence set it back to false to maintain backward compatibility.
Despite the redirect behaviour changed for trailing /, but modern browser should be able to pick up the 302 status code and perform redirect accordingly, hence there is no impact on user level.
How was this patch tested?
Junit test case.
Was this patch authored or co-authored using generative AI tooling?
No