flyrain commented on code in PR #367:
URL: https://github.com/apache/polaris/pull/367#discussion_r1809186459
##########
polaris-service/src/test/java/org/apache/polaris/service/PolarisApplicationIntegrationTest.java:
##########
@@ -103,6 +106,11 @@
SnowmanCredentialsExtension.class
})
public class PolarisApplicationIntegrationTest {
+ @TempDir private static Path tempDir;
+ private static Supplier<String> CURRENT_LOG = () ->
tempDir.resolve("application.log").toString();
+ private static Supplier<String> ARCHIVED_LOG =
+ () -> tempDir.resolve("application-%d-%i.log.gz").toString();
Review Comment:
Looks like the archive logs are not used anywhere. Do we need this change?
##########
polaris-service/src/test/java/org/apache/polaris/service/PolarisApplicationIntegrationTest.java:
##########
@@ -115,7 +123,11 @@ public class PolarisApplicationIntegrationTest {
"server.applicationConnectors[0].port",
"0"), // Bind to random port to support parallelism
ConfigOverride.config(
- "server.adminConnectors[0].port", "0")); // Bind to random port
to support parallelism
+ "server.adminConnectors[0].port", "0"), // Bind to random port
to support parallelism
+ ConfigOverride.config("logging.appenders[1].type", "file"),
+ ConfigOverride.config("logging.appenders[1].currentLogFilename",
CURRENT_LOG),
+
ConfigOverride.config("logging.appenders[1].archivedLogFilenamePattern",
ARCHIVED_LOG),
+ ConfigOverride.config("logging.appenders[1].maxFileSize",
"2000000"));
Review Comment:
Just curious, is 2M big enough? Another option is to check archived log as
well.
##########
polaris-service/src/test/java/org/apache/polaris/service/PolarisApplicationIntegrationTest.java:
##########
@@ -103,6 +106,11 @@
SnowmanCredentialsExtension.class
})
public class PolarisApplicationIntegrationTest {
+ @TempDir private static Path tempDir;
+ private static Supplier<String> CURRENT_LOG = () ->
tempDir.resolve("application.log").toString();
Review Comment:
Can we make it a final field?
##########
polaris-service/src/test/java/org/apache/polaris/service/PolarisApplicationIntegrationTest.java:
##########
@@ -179,6 +198,14 @@ public static void deletePrincipalRole() {
.close();
}
+ private static void assertZeroErrorsInApplicationLog() {
Review Comment:
Do we need to check archived log file as well?
--
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]
