nevzheng commented on PR #13057: URL: https://github.com/apache/gravitino/pull/13057#issuecomment-5649840336
Hi @jerryshao, thanks for the review comments. PTAL. Here's what I changed: - **Iceberg REST and Lance REST** now honor `includeErrorStackTrace` from their own config (`gravitino.iceberg-rest.*`, `gravitino.lance-rest.*`), and the option is documented for both. - **Authentication filter** logs unexpected failures once at ERROR, so the cause stays in the server log when stacks are hidden; 401/403/400 client errors aren't logged. - **`createAuthenticationFilter`** now takes `includeErrorStackTrace`, with a documented contract; the Iceberg and Lance auth error bodies are tested to carry no stack. - **Tests** cover Jetty's error page in both directions, plus Javadoc on the config entry and a single place that resolves the default. - The default stays `true` for compatibility, as we discussed. The PR description is updated to match. Here are some follow-ups we need to consider. Let me know how you'd like to prioritize them: - #13107: authentication filter and `ErrorExceptionMapper` return raw exception messages - #13108: skip building stack traces when the setting is disabled - #13109: stack trace logging for client-driven 403s in `GravitinoInterceptionService` - #13110: correlate error responses with server logs via request IDs (the remaining conditions from #12728) -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
