dimas-b commented on code in PR #1023:
URL: https://github.com/apache/polaris/pull/1023#discussion_r1960942842
##########
build-logic/src/main/kotlin/polaris-java.gradle.kts:
##########
@@ -82,7 +82,7 @@ testing {
dependencies {
implementation(project())
implementation(testFixtures(project()))
- runtimeOnly(
+ implementation(
Review Comment:
why is this change required? Why not add it as an explicit "test" dependency
only where it is truly needed?
##########
service/common/src/main/java/org/apache/polaris/service/exception/IcebergExceptionMapper.java:
##########
@@ -75,6 +75,8 @@ public IcebergExceptionMapper() {}
@Override
public Response toResponse(RuntimeException runtimeException) {
LOGGER.info("Handling runtimeException {}", runtimeException.getMessage());
+ LOGGER.debug("Full runtimeException", runtimeException);
Review Comment:
Would you mind doing this for 500 errors as `.info`?
##########
service/common/src/test/java/org/apache/polaris/service/exception/IcebergExceptionMapperTest.java:
##########
@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.polaris.service.quarkus;
+package org.apache.polaris.service.exception;
Review Comment:
nit: ideally simple renames/moves should be done in isolated PRs... it does
not look like this change is related to the logging changes in other classes :)
--
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: issues-unsubscr...@polaris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
