You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/04/19 03:31:55 UTC
[GitHub] [iceberg] felixYyu opened a new pull request, #4584: logger: SLF4J logger instead System.out and System.err
felixYyu opened a new pull request, #4584:
URL: https://github.com/apache/iceberg/pull/4584
in checkstyle.xml, `Logging with System.out and System.err are not allowed because it has no metadata and can't be configured at runtime. Please use an SLF4J logger instead`. so use SLF4J logger instead System.out and System.err.
cc: @RussellSpitzer @rdblue
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] felixYyu commented on a diff in pull request #4584: System Printing: Removing System.out and System.err
Posted by GitBox <gi...@apache.org>.
felixYyu commented on code in PR #4584:
URL: https://github.com/apache/iceberg/pull/4584#discussion_r853641957
##########
spark/v2.4/spark/src/test/java/org/apache/iceberg/spark/data/TestParquetAvroReader.java:
##########
@@ -116,8 +120,8 @@ public void testStructSchema() throws IOException {
long end = System.currentTimeMillis();
long duration = end - start;
- System.err.println("XOR val: " + val);
- System.err.println(String.format("Reassembled %d records in %d ms", count, duration));
+ LOG.error("XOR val: {}", val);
+ LOG.error("Reassembled {} records in {} ms", count, duration);
Review Comment:
Although this is ignored, but I also think this should be removed.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] felixYyu commented on pull request #4584: System Printing: Removing System.out and System.err
Posted by GitBox <gi...@apache.org>.
felixYyu commented on PR #4584:
URL: https://github.com/apache/iceberg/pull/4584#issuecomment-1105109427
cc @RussellSpitzer can you take a look again, thanks.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a diff in pull request #4584: logger: SLF4J logger instead System.out and System.err
Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #4584:
URL: https://github.com/apache/iceberg/pull/4584#discussion_r853243723
##########
api/src/test/java/org/apache/iceberg/types/TestReadabilityChecks.java:
##########
@@ -362,7 +365,7 @@ public void testStructWriteReordering() {
List<String> errors = CheckCompatibility.writeCompatibilityErrors(read, write);
Assert.assertEquals("Should produce 1 error message", 1, errors.size());
- System.err.println(errors);
Review Comment:
Printing in tests should be removed rather than logged.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] felixYyu commented on a diff in pull request #4584: System Printing: Removing System.out and System.err
Posted by GitBox <gi...@apache.org>.
felixYyu commented on code in PR #4584:
URL: https://github.com/apache/iceberg/pull/4584#discussion_r853640863
##########
api/src/test/java/org/apache/iceberg/types/TestReadabilityChecks.java:
##########
@@ -362,7 +365,7 @@ public void testStructWriteReordering() {
List<String> errors = CheckCompatibility.writeCompatibilityErrors(read, write);
Assert.assertEquals("Should produce 1 error message", 1, errors.size());
- System.err.println(errors);
Review Comment:
removed
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a diff in pull request #4584: logger: SLF4J logger instead System.out and System.err
Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #4584:
URL: https://github.com/apache/iceberg/pull/4584#discussion_r853244577
##########
spark/v2.4/spark/src/test/java/org/apache/iceberg/spark/data/TestParquetAvroReader.java:
##########
@@ -116,8 +120,8 @@ public void testStructSchema() throws IOException {
long end = System.currentTimeMillis();
long duration = end - start;
- System.err.println("XOR val: " + val);
- System.err.println(String.format("Reassembled %d records in %d ms", count, duration));
+ LOG.error("XOR val: {}", val);
+ LOG.error("Reassembled {} records in {} ms", count, duration);
Review Comment:
This is ignored, so there is no need to update this. It is for sanity checking the implementation.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer merged pull request #4584: System Printing: Removing System.out and System.err
Posted by GitBox <gi...@apache.org>.
RussellSpitzer merged PR #4584:
URL: https://github.com/apache/iceberg/pull/4584
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org