Skip to content

Conversation

namanagraw
Copy link

What changes were proposed in this pull request?

  1. Upgraded jetty to 10.0.1
  2. Upgraded javax.servlet-api.version to 4.0.1

How was this patch tested?

Tested in local
image

@brahmareddybattula
Copy link

thanks @namanagraw for reporting . Changes lgtm. thanks for providing the validation report also. Need to improve UT and please track UT's once after this commit.

@ksumit ksumit requested a review from Copilot August 2, 2025 06:18
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR upgrades the Jetty web server dependency from version 9.4.50.v20221201 to 10.0.1 and updates the servlet API version from 3.1.0 to 4.0.1. The upgrade requires updating the request logging implementation to use Jetty 10's new API.

  • Updated Jetty version from 9.4.50 to 10.0.1 in Maven dependencies
  • Updated javax.servlet-api from 3.1.0 to 4.0.1 to match Jetty 10 requirements
  • Refactored request logging to use Jetty 10's new CustomRequestLog and RequestLogWriter APIs

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
pom.xml Updates Jetty and servlet API version properties
WebServer.scala Migrates from deprecated NCSARequestLog to new CustomRequestLog implementation
Comments suppressed due to low confidence (1)

server/src/main/scala/org/apache/livy/server/WebServer.scala:101

  • The setExtended(false) method call was removed during the migration to CustomRequestLog, but this configuration might be needed. In Jetty 10, extended logging is controlled by the log format string passed to CustomRequestLog constructor. Verify that the NCSA_FORMAT provides the desired level of logging detail.
  requestLogWriter.setTimeZone("GMT")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants