[GH-2744] Clean up Maven build logs#2801
Merged
Merged
Conversation
- Fix Scala compiler warnings: remove redundant import in Moran.scala, unreachable code in GeoParquetMetaData.scala, add missing match cases in BroadcastIndexJoinExec.scala and JoinQueryDetector.scala, fix uninitialized variable order in ScalaExample.scala - Replace System.out.println with SLF4J/Log4j loggers across test and main source files to eliminate console noise during builds - Raise test log4j2 root logger level from info to warn across all modules (common, spark, flink)
Contributor
There was a problem hiding this comment.
Pull request overview
This PR reduces Maven build console noise by routing ad-hoc System.out output through logging frameworks and by raising test log levels, while also addressing several Scala compiler warnings (mostly around exhaustiveness/unreachable code).
Changes:
- Raise test Log4j2 root logger level from
infotowarnacross Spark/Flink/common modules. - Replace
System.out.println/printfdebug prints with SLF4J/Log4j logging across tests and example/demo code. - Fix Scala compiler warnings (e.g., missing match cases, redundant import, unreachable code cleanup).
Reviewed changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| spark/spark-4.1/src/test/resources/log4j2.properties | Raise test root logging level to warn. |
| spark/spark-4.0/src/test/resources/log4j2.properties | Raise test root logging level to warn. |
| spark/spark-3.5/src/test/resources/log4j2.properties | Raise test root logging level to warn. |
| spark/spark-3.4/src/test/resources/log4j2.properties | Raise test root logging level to warn. |
| spark/common/src/test/resources/log4j2.properties | Raise test root logging level to warn. |
| spark/common/src/test/java/org/apache/sedona/viz/NYCTripPointMapper.java | Replace stdout printing with SLF4J logging in test helper. |
| spark/common/src/test/java/org/apache/sedona/core/spatialPartitioning/quadtree/RenderQuadTree.java | Replace stdout printing with SLF4J logging in test UI helper. |
| spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/strategy/join/JoinQueryDetector.scala | Add missing match cases to remove Scala exhaustiveness warnings. |
| spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/strategy/join/BroadcastIndexJoinExec.scala | Add missing match case to remove Scala warnings in plan string rendering. |
| spark/common/src/main/scala/org/apache/spark/sql/execution/datasources/geoparquet/GeoParquetMetaData.scala | Simplify mapField logic to avoid Scala warnings. |
| spark/common/src/main/scala/org/apache/sedona/viz/showcase/ScalaExample.scala | Reorder initialization and reduce direct stdout noise (still uses println). |
| spark/common/src/main/scala/org/apache/sedona/stats/autocorrelation/Moran.scala | Remove redundant import to fix Scala warning. |
| spark/common/src/main/scala/org/apache/sedona/core/showcase/ScalaExample.scala | Replace System.out.println with println. |
| spark/common/src/main/scala/org/apache/sedona/core/showcase/ScalaEarthdataMapperRunnableExample.scala | Replace System.out.println with println. |
| spark/common/src/main/java/org/apache/sedona/viz/showcase/Example.java | Replace stdout printing with Log4j logging in demo. |
| spark/common/src/main/java/org/apache/sedona/viz/extension/visualizationEffect/ChoroplethMap.java | Replace stdout printing with existing logger calls. |
| spark/common/src/main/java/org/apache/sedona/viz/core/ImageSerializableWrapper.java | Replace stdout printing with Log4j warning. |
| spark/common/src/main/java/org/apache/sedona/core/showcase/Example.java | Replace stdout printing with Log4j logging in demo. |
| spark/common/src/main/java/org/apache/sedona/core/showcase/EarthdataMapperRunnableExample.java | Replace stdout printing with Log4j logging in demo. |
| spark/common/src/main/java/org/apache/sedona/core/formatMapper/shapefileParser/ShapefileRDD.java | Replace stdout printing with SLF4J logging. |
| snowflake/src/main/java/org/apache/sedona/snowflake/snowsql/ddl/DDLGenerator.java | Replace stdout printing with SLF4J logging for CLI usage + DDL output. |
| snowflake-tester/src/test/java/org/apache/sedona/snowflake/snowsql/TestBase.java | Replace stdout printing with test logger output. |
| snowflake-tester/src/test/java/org/apache/sedona/snowflake/snowsql/SnowTestRunner.java | Replace stdout printing with SLF4J logging. |
| snowflake-tester/src/test/java/org/apache/sedona/snowflake/snowsql/SnowClient.java | Replace stdout printing with structured logger output. |
| flink/src/test/resources/log4j2.properties | Raise test root logging level to warn. |
| flink/src/test/java/org/apache/sedona/flink/FunctionTest.java | Replace stdout printing with SLF4J debug logging. |
| common/src/test/resources/log4j2.properties | Raise test root logging level to warn. |
| common/src/test/java/org/apache/sedona/common/geometrySerde/SpatialIndexSerdeTest.java | Replace stdout printing with SLF4J logging in tests. |
| common/src/test/java/org/apache/sedona/common/S2Geography/TestHelper.java | Replace stdout printing with SLF4J debug logging. |
| common/src/test/java/org/apache/sedona/common/FunctionsTest.java | Replace stdout printing with SLF4J debug logging. |
| common/src/test/java/org/apache/sedona/common/FunctionsProj4PerformanceTest.java | Replace stdout/printf with SLF4J logging in performance tests. |
Comments suppressed due to low confidence (1)
spark/common/src/main/scala/org/apache/sedona/viz/showcase/ScalaExample.scala:52
ConfFileis aFileInputStreamthat is repeatedly reassigned and passed toprop.load(...)but never closed. This can leak file descriptors in long-running demo runs; consider using a scoped resource pattern (e.g.,scala.util.Using) or explicitly closing the stream after eachprop.load.
val resourcePath = "/../spark/common/src/test/resources/"
var ConfFile = new FileInputStream(resourcePath + "babylon.point.properties")
val demoOutputPath = "target/demo"
val scatterPlotOutputPath =
System.getProperty("user.dir") + "/" + demoOutputPath + "/scatterplot"
prop.load(ConfFile)
val heatMapOutputPath = System.getProperty("user.dir") + "/" + demoOutputPath + "/heatmap"
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Revert DDL SQL output lines back to System.out.println since the tool is used with stdout redirection (java -jar ... > sedona-snowflake.sql) - Replace assert with explicit check for required geotools-version arg (assertions are disabled by default without -ea) - Fix usage text: --h -> -h to match the actual argument parser
The snowflake DDLGenerator is a CLI tool that outputs SQL to stdout for file redirection. All println usage in snowflake and snowflake-tester should remain unchanged.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
compareTo(geoWKT, geoWKT) was comparing the same object, making the assertion always pass. Changed to compareTo(geoWKB, geoWKT).
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Replace placeholder match arms with explicit exceptions: - Raster + distance: createStreamShapes deserializes as geometry, would fail on raster bytes - Geography + non-raster: not yet implemented - Geography + raster: not yet implemented
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The compareTo(geoWKT, geoWKT) bug masks a deeper issue: WKB and WKT readers produce incompatible Geography types for empty geometries, causing compareTo to fail. Reverting to unblock CI; tracked in a separate issue.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
System.out.printlnwith SLF4J/Log4j loggers across test and main source files to eliminate console noise during Maven buildsinfotowarnacross all modules (common, spark, flink)Test plan
Closes #2744