Skip to content

Fix GeoWave Tests - #1665

Merged
lossyrob merged 5 commits into
locationtech:masterfrom
jamesmcclain:fix/geowave-tests
Oct 10, 2016
Merged

Fix GeoWave Tests#1665
lossyrob merged 5 commits into
locationtech:masterfrom
jamesmcclain:fix/geowave-tests

Conversation

@jamesmcclain

@jamesmcclain jamesmcclain commented Oct 7, 2016 •
edited
Loading

Copy link
Copy Markdown
Member

This allows the tests in the GeoWave subproject to pass. Because there is currently an incompatibility with the latest versions of GeoWave, an older version of GeoWave must (currently) be built and installed in one's local ~/.m2 repository. The complete sequence is:

  1. Build an older version of GeoWave locally. That can be done by pulling down this other repository and typing make world.
  2. Apply the diff listed at the bottom of this comment to this branch
  3. Type docker run -td --restart=always --net=geowave -p 50095:5009 -p 21810:2181 -p 9997:9997 -p 9999:9999 --hostname leader --name leader jamesmcclain/geowave:c127c16 to start a local GeoWave instance
  4. Type echo leader localhost > /tmp/hostaliases to create a host aliases file
  5. Type HOSTALIASES=/tmp/hostaliases ./sbt -J-Xmx2G "project geowave" clean test to run the tests
diff --git a/geowave/build.sbt b/geowave/build.sbt
index a89620b..db5d747 100644
--- a/geowave/build.sbt
+++ b/geowave/build.sbt
@@ -50,11 +50,11 @@ libraryDependencies ++= Seq(
   scalatest % "test")

 resolvers ++= Seq(
-  // Resolver.mavenLocal,
+  Resolver.mavenLocal,
   "boundless" at "https://repo.boundlessgeo.com/release",
   "geosolutions" at "http://maven.geo-solutions.it/",
-  "geowave-release" at "http://geowave-maven.s3-website-us-east-1.amazonaws.com/release",
-  "geowave-snapshot" at "http://geowave-maven.s3-website-us-east-1.amazonaws.com/snapshot",
+  // "geowave-release" at "http://geowave-maven.s3-website-us-east-1.amazonaws.com/release",
+  // "geowave-snapshot" at "http://geowave-maven.s3-website-us-east-1.amazonaws.com/snapshot",
   "osgeo" at "http://download.osgeo.org/webdav/geotools/"
 )

pomadchin
pomadchin reviewed Oct 8, 2016
View reviewed changes
Comment thread geowave/src/main/scala/geotrellis/spark/io/kryo/GeowaveKryoRegistrator.scala Outdated
* SimpleFeatureType serializer. This makes use of the
* encoding/decoding machinery provied by GeoTools.
*/
private class SimpleFeatureTypeSerializer extends Serializer[SimpleFeatureType] {

@pomadchin pomadchin Oct 8, 2016 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Iin fact SimpleFeatureType is serializable well itself though requires UnmodifiableCollections kryo serializer. I did so in a GeoMesa subproject. Mb it's a good idea to put this SimpleFeatureTypeSerializer into geotools project, or somewhere to be available from both GeoWave and GeoMesa subprojects?

@jamesmcclain jamesmcclain Oct 9, 2016

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the pointer, I will give that a try.

@jamesmcclain jamesmcclain Oct 10, 2016

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to remove the custom serializer and replace it with the javakaffe one.

pomadchin
pomadchin reviewed Oct 8, 2016
View reviewed changes
Comment thread geowave/src/test/scala/geotrellis/spark/io/geowave/GeoWaveFeatureRDDReaderSpec.scala
val featureRDD = sc.parallelize(features)
val zookeeper = "localhost:20000"
val instanceName = "AccumuloInstance"
val zookeeper = "localhost:21810"

@pomadchin pomadchin Oct 8, 2016 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use Accumulo Mock instance? (iterators would be available, if they would be included as a dep in tests)

@jamesmcclain jamesmcclain Oct 9, 2016

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be great, but unfortunately I was not able to get that to work. It is sufficient to allow the vector and attribute store tests to run, but not the raster ones.

@jamesmcclain jamesmcclain mentioned this pull request Oct 10, 2016
Merged
4 tasks done
@lossyrob

lossyrob commented Oct 10, 2016

Copy link
Copy Markdown
Member

+1, can you create an issue that references this PR and the GeoWave issue, that keeps track of changing the tests back to using a not-old GeoWave once that issue gets resolved?

@jamesmcclain jamesmcclain mentioned this pull request Oct 10, 2016
Closed
@jamesmcclain

jamesmcclain commented Oct 10, 2016

Copy link
Copy Markdown
Member Author

This PR does not actually change the version of GeoWave that is used.

When one builds a compatible version of GeoWave locally and applies the diff at bottom of the PR description, then tests run successfully. I did not include the diff in PR, because it is my hope that some future version of GeoWave will again be compatible and no changes will be needed. The code and the tests compile with the current GeoWave snapshot, they just do not function.

I created an issue #1669 to keep track of that.

@lossyrob
lossyrob merged commit 40a2f7a into locationtech:master Oct 10, 2016
@jamesmcclain
jamesmcclain deleted the fix/geowave-tests branch October 10, 2016 18:47
@lossyrob lossyrob added this to the 1.0 milestone Oct 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pomadchin pomadchin pomadchin left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

1.0

Development

Successfully merging this pull request may close these issues.

3 participants

@jamesmcclain @lossyrob @pomadchin