AWS, Core: Switch Jetty to use new Compression API for GZIP#15043
AWS, Core: Switch Jetty to use new Compression API for GZIP#15043nastra merged 3 commits intoapache:mainfrom
Conversation
nastra
force-pushed
the
jetty-switch-to-new-compression-api
branch
2 times, most recently
from
d365a5f to
e935da0
Compare
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions. |
nastra
force-pushed
the
jetty-switch-to-new-compression-api
branch
from
29edaf8 to
91f00ef
Compare
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions. |
|
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time. |
|
@nastra can you please rebase this PR? |
nastra
force-pushed
the
jetty-switch-to-new-compression-api
branch
2 times, most recently
from
5e6e2bc to
eec9e28
Compare
nastra
force-pushed
the
jetty-switch-to-new-compression-api
branch
from
eec9e28 to
14cba3f
Compare
| jakarta-el-api = { module = "jakarta.el:jakarta.el-api", version.ref = "jakarta-el-api" } | ||
| jakarta-servlet = {module = "jakarta.servlet:jakarta.servlet-api", version.ref = "jakarta-servlet-api"} | ||
| jetty-server = { module = "org.eclipse.jetty:jetty-server", version.ref = "jetty" } | ||
| jetty-server = { module = "org.eclipse.jetty.compression:jetty-compression-server", version.ref = "jetty" } |
There was a problem hiding this comment.
This might be confusing. I'd suggest naming it jetty-compression-server and jetty-compression-gzip below.
There was a problem hiding this comment.
+1, My agent review flagged this and noted that the Iceberg-AWS module was only working properly because the "normal server" is a transitive dependency of this module.
| @@ -76,8 +75,6 @@ public void before() throws Exception { | |||
| new ServletContextHandler(ServletContextHandler.NO_SESSIONS); | |||
| servletContext.addServlet( | |||
| new ServletHolder(new RESTCatalogServlet(adapterForRESTServer)), "/*"); | |||
There was a problem hiding this comment.
I would probably keep the compression here and just make a special servlet just for the freshness test. Would it make sense to just override the setup for TestFreshnessAwareLoading specifically?
There was a problem hiding this comment.
good point, yeah I think that's a good idea
RussellSpitzer
left a comment
RussellSpitzer
left a comment
There was a problem hiding this comment.
I have a few nits, but this looks good to me.
nastra
force-pushed
the
jetty-switch-to-new-compression-api
branch
from
d6399bf to
7dcaafb
Compare
|
I'll merge this since the failing Flink CI job is unrelated. Thanks for the reviews @manuzhang and @RussellSpitzer |
nastra
changed the title
3 participants
This currently depends on Build: Bump Jetty to 12.1.5