Merge pull request locationtech#345 from s22s/fix/267

Fixed Exception swallowing in RasterSource.
s22s · Sep 13, 2019 · 6e78377 · 6e78377
2 parents d183c81 + 19b9573
commit 6e78377
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 10 deletions.
diff --git a/...scala/org/locationtech/rasterframes/expressions/generators/RasterSourceToRasterRefs.scala b/...scala/org/locationtech/rasterframes/expressions/generators/RasterSourceToRasterRefs.scala
@@ -21,7 +21,6 @@
 
 package org.locationtech.rasterframes.expressions.generators
 
-import com.typesafe.scalalogging.LazyLogging
 import geotrellis.raster.GridBounds
 import geotrellis.vector.Extent
 import org.apache.spark.sql.catalyst.InternalRow
@@ -45,7 +44,7 @@ import scala.util.control.NonFatal
  * @since 9/6/18
  */
 case class RasterSourceToRasterRefs(children: Seq[Expression], bandIndexes: Seq[Int], subtileDims: Option[TileDimensions] = None) extends Expression
-  with Generator with CodegenFallback with ExpectsInputTypes with LazyLogging {
+  with Generator with CodegenFallback with ExpectsInputTypes {
 
   override def inputTypes: Seq[DataType] = Seq.fill(children.size)(RasterSourceType)
   override def nodeName: String = "rf_raster_source_to_raster_ref"
@@ -77,9 +76,10 @@ case class RasterSourceToRasterRefs(children: Seq[Expression], bandIndexes: Seq[
     }
     catch {
       case NonFatal(ex) ⇒
-        val payload = Try(children.map(c => RasterSourceType.deserialize(c.eval(input)))).toOption.toSeq.flatten
-        logger.error("Error fetching data for one of: " + payload.mkString(", "), ex)
-        Traversable.empty
+        val description = "Error fetching data for one of: " +
+          Try(children.map(c => RasterSourceType.deserialize(c.eval(input))))
+            .toOption.toSeq.flatten.mkString(", ")
+        throw new java.lang.IllegalArgumentException(description, ex)
     }
   }
 }

diff --git a/core/src/test/scala/org/locationtech/rasterframes/ref/RasterRefSpec.scala b/core/src/test/scala/org/locationtech/rasterframes/ref/RasterRefSpec.scala
@@ -21,8 +21,11 @@
 
 package org.locationtech.rasterframes.ref
 
+import java.net.URI
+
 import geotrellis.raster.{ByteConstantNoDataCellType, Tile}
 import geotrellis.vector.Extent
+import org.apache.spark.SparkException
 import org.apache.spark.sql.Encoders
 import org.locationtech.rasterframes.{TestEnvironment, _}
 import org.locationtech.rasterframes.expressions.accessors._
@@ -205,6 +208,16 @@ class RasterRefSpec extends TestEnvironment with TestData {
         r.rows should be <= NOMINAL_TILE_SIZE
       }
     }
+    it("should throw exception on invalid URI") {
+      val src = RasterSource(URI.create("http://foo/bar"))
+      import spark.implicits._
+      val df = Seq(src).toDF("src")
+      val refs = df.select(RasterSourceToRasterRefs($"src") as "proj_raster")
+      logger.warn(Console.REVERSED + "Upcoming 'java.lang.IllegalArgumentException' expected in logs." + Console.RESET)
+      assertThrows[SparkException] {
+        refs.first()
+      }
+    }
   }
 
   describe("RealizeTile") {

diff --git a/docs/src/main/paradox/release-notes.md b/docs/src/main/paradox/release-notes.md
@@ -4,10 +4,13 @@
 
 ### 0.8.2
 
+* Fixed issue with `RasterSourceDataSource` swallowing exceptions. ([#267](https://github.com/locationtech/rasterframes/issues/267)) 
 * Fixed SparkML memory pressure issue caused by unnecessary reevaluation, overallocation, and primitive boxing. ([#343](https://github.com/locationtech/rasterframes/issues/343)) 
 * Fixed Parquet serialization issue with `RasterRef`s ([#338](https://github.com/locationtech/rasterframes/issues/338))
 * Fixed `TileExploder`, `rf_agg_local_mean` and `TileColumnSupport` to support `proj_raster` struct ([#287](https://github.com/locationtech/rasterframes/issues/287), [#163](https://github.com/locationtech/rasterframes/issues/163), [#333](https://github.com/locationtech/rasterframes/issues/333)).
 * Various documentation improvements.
+* _Breaking_ (potentially): Synchronized parameter naming in Python and Scala for `spark.read.raster` ([#329](https://github.com/locationtech/rasterframes/pull/329)).
+
 
 ### 0.8.1
 

diff --git a/pyrasterframes/src/main/python/tests/RasterSourceTest.py b/pyrasterframes/src/main/python/tests/RasterSourceTest.py
@@ -124,11 +124,26 @@ def test_list_of_list_of_str(self):
         self.assertTrue(len(df.columns) == 4)  # 2 cols of uris plus 2 cols of proj_rasters
         self.assertEqual(sorted(df.columns), sorted(['proj_raster_0_path', 'proj_raster_1_path',
                                                      'proj_raster_0', 'proj_raster_1']))
-        uri_df = df.select('proj_raster_0_path', 'proj_raster_1_path').distinct().collect()
-        uri_list = [list(r.asDict().values()) for r in uri_df]
-        self.assertTrue(lol[0] in uri_list)
-        self.assertTrue(lol[1] in uri_list)
-        self.assertTrue(lol[2] in uri_list)
+        uri_df = df.select('proj_raster_0_path', 'proj_raster_1_path').distinct()
+
+        # check that various uri's are in the dataframe
+        self.assertEqual(
+            uri_df.filter(col('proj_raster_0_path') == lit(self.path(1, 1))).count(),
+            1)
+
+        self.assertEqual(
+            uri_df \
+                .filter(col('proj_raster_0_path') == lit(self.path(1, 1))) \
+                .filter(col('proj_raster_1_path') == lit(self.path(1, 2))) \
+                .count(),
+            1)
+
+        self.assertEqual(
+            uri_df \
+                .filter(col('proj_raster_0_path') == lit(self.path(3, 1))) \
+                .filter(col('proj_raster_1_path') == lit(self.path(3, 2))) \
+                .count(),
+            1)
 
     def test_schemeless_string(self):
         import os.path