Skip to content

GeoWave input plugin for Mapnik#2758

Closed
jwomeara wants to merge 1 commit intomapnik:masterfrom
jwomeara:geowave-plugin
Closed

GeoWave input plugin for Mapnik#2758
jwomeara wants to merge 1 commit intomapnik:masterfrom
jwomeara:geowave-plugin

Conversation

@jwomeara
Copy link
Copy Markdown

No description provided.

@jwomeara
Copy link
Copy Markdown
Author

Hello, I referenced this pull request on your google groups page here. Please let me know if you have any questions or comments.

Thanks,
Whitney

@jwomeara jwomeara changed the title Geowave input plugin for Mapnik GeoWave input plugin for Mapnik Apr 10, 2015
@jwomeara
Copy link
Copy Markdown
Author

I updated GeoWave's documentation to explain how to generate the GeoWave Jace proxies. Travis is in the process of updating our docs, but when it finishes it will be available here

EDIT: The link actually works now, doh!

@flippmoke
Copy link
Copy Markdown
Member

Some initial thoughts:

  • Would love to see a way some tests for this as I have no experience with GeoWave, so will be hard if we change any of the core.
  • The inclusion of JVM makes me a little nervous in general. Does this work better if we upgraded the java bindings (https://github.com/springmeyer/mapnik-jni) to actually be up to date? It is so far behind this could be a really big task, though... Would love to see someone support and keep up these bindings! ( @ShibaBandit would be someone great to help.
  • We are planning on updating the geometry backend very soon (see the mapnik-geometry branch), so some of this code will have to be updated soon.
  • Not sure why java is a different plugin here as well, does it make sense to have it all in one, otherwise might be confusing that both are required.

Would love to help in any way I can. Thanks so much for your pull request 👍

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+1.06%) to 77.14% when pulling ba1944a on jwomeara:geowave-plugin into fc91179 on mapnik:master.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+1.06%) to 77.14% when pulling ba1944a on jwomeara:geowave-plugin into fc91179 on mapnik:master.

@jwomeara
Copy link
Copy Markdown
Author

I can absolutely work on creating a test. For projects built on Accumulo (like GeoWave), you can spin up a Mini Accumulo cluster for testing. Using this, you can easily setup a full Accumulo cluster with GeoWave ready to go. That's something that we often use for our unit & integration tests. I'll create Jace bindings for that and writing a quick test!

I should also point out that for PDAL, they have a Vagrantfile which deploys their project to an Ubuntu VM. When we integrated with them we updated their provisioning scripts to setup the GeoWave plugin and to deploy a standalone Mini Accumulo cluster with GeoWave. This was nice because it documents how to setup the plugin and provides a pre-configured environment to play with PDAL and GeoWave. We could do something like that for Mapnik as well. In fact, we liked that idea so much that we stole it and created a Vagrantfile for GeoWave. :)

I checked out the mapnik-jni project that you shared. That definitely seems like the way to go if you have a Java project where your goal is to leverage some existing Mapnik functionality. However, in our case, we are looking to add functionality to Mapnik and enable your users to leverage GeoWave as their backing store. I might be wrong, but it didn't seem like mapnik-jni would allow us to do that.

Speaking specifically to the JVM as a dependency... We decided on using Jace to generate JNI bindings for two reasons.

For one, Jace gives us an automated way of creating robust proxies for our Java classes. Using Jace, we are able to expose the entirety of our API to C++ projects almost exactly as it exists in Java. The method signatures are all the same, and we can even generate bindings for packages which exist outside of GeoWave. Most importantly, using Jace our proxy generation is automated and dead simple, so we can expose additional Java classes as needed on a whim. Not to mention the fact that in the very near future we will be able to provide deb and rpm packaging for our headers/library/jar.

Second, by using JNI we are able to integrate directly with C++ projects. The alternative would be to establish some sort of message passing paradigm. If we took that approach, we would realistically only expose a subset of our overall API. Plus, anytime a C++ project takes GeoWave as a dependency, there would have to be a companion GeoWave task running in the JVM (this functionality is already handled in our plugin and is provided by Jace). Most importantly to us, by going with an approach like this, we are adding additional overhead to pass messages. With JNI, we are able to eliminate message passing overhead and expose our entire API.

I apologize if I came off sounding like a guy on his soap box. We are certainly open to alternative methods of integration, so if there's something you are more comfortable with, please point us to it and we'll check it out. I just wanted to share some of our thoughts about why we decided to go with direct integration with JNI via Jace.

Moving right along... I added a separate plugin for the Java API, but that can easily be removed if you want. My thought process here was that Java should stand on it's own because it's not something that's really owned by GeoWave and is a pretty major dependency in it's own right. I should point out that when using JNI, you can only create one JVM instance for the life of the host application. So, once you close the connection to the JVM, you can't instantiate a new one. By having that plugin separate from GeoWave, it lets other Java projects know that there is a precedent for integrating Java with Mapnik, and hopefully encourages them to play nice with the other JNI datasources and not do something which would completely hose the JVM.

Whoo! Sorry for the wall of text.

I think I have at least one action item to build in a test using Mini Accumulo Cluster. Does that sound good to you for now?

@chrisbennight
Copy link
Copy Markdown

@flippmoke
@jwomeara I think covered pretty much everything, but for a short summary the use case here is much closer to the Python plugin (rational basically mimics our thought process here: https://github.com/mapnik/mapnik/tree/master/plugins/input/python#rationale ).

Re: hesitancy to take on the JVM as a dependency;
Completely get the concern - the hope here is that people can completely ignore this if it's not relevant to their build profile (with the expectation that even if they do a build all plugins but don't have the JDK on their box scons will just skip it).
On the plus side this established a pattern for inter-oping with other JVM based stacks in the future (i.e. a GeoTools data store bridge, allowing for some interesting cross pollination.)

Re: testing
We would provide self contained unit tests in the spirit of the python ones; the JVM would be required on the test machine; otherwise we would download everything in a self contained and automated manner (i.e. no setup required). This is a pattern we use both internally and for integration with other projects.

@jwomeara
Copy link
Copy Markdown
Author

@flippmoke

I see that some changes have been made to the Mapnik layout. I made some updates to our geowave plugin and added a python test. However, I see that the python bindings and tests have been moved to their own repo.

Should I just move the python test over and create a separate pull request?

Also, any feedback on what we've written so far would be greatly appreciated!

Thanks!

@jwomeara
Copy link
Copy Markdown
Author

Also, I see that I'm a bit behind master now (about 355 commits, ha!). I'll rebase tomorrow, but before I finalize that, I want to make sure I have a home for our python test.

Thanks again.

@flippmoke
Copy link
Copy Markdown
Member

@jwomeara So things are going to get a little crazy for this plugin, sorry again that you are trying to hit a fast moving target right now. So... python bindings and tests got moved out of mapnik core and now are here:
https://github.com/mapnik/python-mapnik

I have had some conversations with @springmeyer recently about how we can better support the brave developers like you who are developing input plugins. Some plugins might remain in mapnik-core because they are very common, but we would like a way to separate them out into different repositories. The goal of this would be the ability to have plugin development and core development separate, but have testing platforms to check if plugins stay up to date with mapnik 3.x. This would be somewhat similar to the idea behind https://github.com/mapbox/mapnik-swoop but hopefully much easier.

In short we want to make it easier for developers to build plugins against versioned numbers of mapnik 3.x (once we get everything in order that we can make the first official 3.0). In some ways, pardon the mess currently, we really are trying to make it easier on you! Thanks again for trying to contribute.

@jwomeara
Copy link
Copy Markdown
Author

jwomeara commented May 4, 2015

Thanks for the response. At the moment, I'm having trouble building the python-mapnik project. I'm guessing that the geometry updates haven't made their way over to the python bindings yet. The build is failing on mapnik_geometry.cpp, line 155 on a call to is_valid. I think this was a method that was available for the previous geometry implementation, but has since disappeared with the new implementation.

Anyway, should I put python integration on hold for the moment? Should I make a pull request? The python bindings were working before the big update. Also, my plugin is building correctly within the core project after rebasing with master. I'm guessing you'd like to see that code ported over to your non-core-plugins project, right? If so, I can work on that next.

As always, thanks!

@jwomeara
Copy link
Copy Markdown
Author

jwomeara commented May 5, 2015

Scratch my previous comment. It appears that some of the geometry methods are unavailable when using a boost version less than 1.56. I'm going to update boost and try again.

@flippmoke
Copy link
Copy Markdown
Member

@jwomeara you might look into using bootstrap.sh to help setup an environment for you, its different then having everything system installed as it just downloads the binaries you need.

@springmeyer do we have good docs for that process yet?

@jwomeara
Copy link
Copy Markdown
Author

jwomeara commented May 5, 2015

I have everything running now. I'm just running through my python tests
again and making sure that I am handling geometry correctly with the latest
changes. I'm hoping to have an updated pull request by COB.

Thanks for everything so far!

On Tue, May 5, 2015 at 1:14 PM, Blake Thompson notifications@github.com
wrote:

@jwomeara https://github.com/jwomeara you might look into using
bootstrap.sh to help setup an environment for you, its different then
having everything system installed as it just downloads the binaries you
need.

@springmeyer https://github.com/springmeyer do we have good docs for
that process yet?


Reply to this email directly or view it on GitHub
#2758 (comment).

@jwomeara
Copy link
Copy Markdown
Author

jwomeara commented May 5, 2015

Ok, so I'm trying to hit this moving target again. :)

I should be up to date with you guys now. I made updates to support the geometry changes that were made. I also factored the python tests out and made a pull request here.

The python integration test is running correctly for me. I also played around a little with your basic mapnik example and made sure that I am able to correctly generate a map from one of the shapefiles from test/data.

As always, let me know if you have any questions/comments. I'll try to implement any requested changes as soon as I am able.

Thanks for all of your help so far!

@jwomeara
Copy link
Copy Markdown
Author

jwomeara commented May 5, 2015

Ok, I have one more thing I have to do. We did some refactoring on our side, so a bunch of the GeoWave header locations will need to be changed. Once I figure that out, I'll make a final update to this pull request. As it stands right now, this pull request should still be valid assuming that you are using GeoWave v0.8.6.

@jwomeara jwomeara force-pushed the geowave-plugin branch 2 times, most recently from 9d521a5 to f954281 Compare May 7, 2015 14:33
@jwomeara
Copy link
Copy Markdown
Author

jwomeara commented May 7, 2015

Everything should be good to go now. Please, let me know if you have any questions!

@jwomeara
Copy link
Copy Markdown
Author

@flippmoke
@springmeyer

Hey guys,

With my latest updates to this branch, I added support for configuring GeoWave through your Mason repo. Unfortunately, I cannot do a pull request to add my geowave-jace-0.8.7 branch to the mapbox/mason repo. However, you can find my fork of your mason repo here, and if one of you is happy with my changes on the geowave-jace-0.8.7 branch here, then perhaps you could add this branch to your mason repo.

We cut a new release (0.8.7) to support integration with Mapnik. I also put in some extra effort to ensure that geowave-jace bindings are created every time we create a new release. Additionally, I made sure that everything compiles and runs correctly whether you use gcc or clang.

I also updated your travis config to build with the GeoWave plugin. Java 7 is already included on Travis' c++ machines, so all I had to do was add a JAVA_HOME environment variable and add the jvm lib to LD_LIBRARY_PATH. I also updated your bootstrap script to install GeoWave from Mason. Testing locally, everything compiles and runs perfectly.

Anyway, I just wanted to give you guys an update. We're still working hard to hit this moving target and are interested in hearing what you guys think of what we've done so far. If there's anything else we can do to facilitate the integration effort, please let us know. We are definitely motivated to make integration as simple as possible for you and your users.

To recap, we have:

  • geowave-jace-0.8.7 branch for mason here
  • geowave-plugin branch for Mapnik here
  • geowave-plugin branch for python-mapnik here

As always, thank you, and if you have any questions or comments for me or the other GeoWave developers, please let us know.

  • Whitney

@flippmoke
Copy link
Copy Markdown
Member

@jwomeara Love what you guys are doing, we are working hard right now to get 3.0.0 Released and then will go back to addressing some of the pull requests like this plugin. We are considering still how is the best way to make plugins that are external to core, and want to provide examples. This is something that @springmeyer and I have been talking about. Basically we could help you get a mapnik/geowave-plugin repo setup. This would be ideal because we want to make it easy for people to add plugins on demand, and we are even looking at the current plugins and thinking about if any of them need to be pulled out into seperate repos.

We know that this is a little more challenging in some ways for deployment of the plugins, but we are working hard on thinking about good ways and good tests for all plugins as we release new versions of mapnik. The great part about having a plugin outside of "mapnik core" is that it can be quickly iterated up on with out relying on a new release of mapnik. We think this will be important as we want have stable release of "mapnik core".

Additionally we are looking at a complete makeover of the mapnik.org site, on it we want to add plugin documentation as well, so that people aren't confused on how to use certain plugins. Once we have that up and running it should be something similar to adding markdown.

In the mean time please look at https://github.com/mapnik/hello-world-input-plugin and I have created a new team https://github.com/orgs/mapnik/teams/geowave and repo https://github.com/mapnik/geowave-plugin for you! 👍

I am sorry that all of this is a big PITA, but you guys are kind of our testers for a new way of organizing everything. We really appreciate your hard work. If you want to coordinate more with us in any way or voice chat sometime, let me know as we want to make sure and help people like you.

@flippmoke
Copy link
Copy Markdown
Member

@jwomeara wanted to ping you on this to see how things were going, saw you didn't accept invitation to join geowave team of mapnik

@jwomeara
Copy link
Copy Markdown
Author

Hey @flippmoke,

Sorry for the going dark over the past two weeks. Two weeks ago, my wife brought our second child into the world. One week ago, we packed up and moved to a new state. We almost have everything unpacked and I'm just starting to go back to work (albeit at a new location where I am still waiting for computer access. Ugh!)

Anyway, I'm just starting to get back into the swing of things. I'm very happy with your response to our latest pull request. I would love for our plugin to be a part of "mapnik core," but I definitely understand your reasoning for wanting plugins to exist independently. The biggest concern I have going forward is just to make sure that the plugins stay up to date with mapnik core and vice versa. Tying the plugins to a specific release is probably the best solution to that problem. Do you guys want to have the python bindings separated out as well, or can we contribute directly to python mapnik?

W.R.T. documentation, we can definitely cook something up. We already have some documentation on our end which describes how to get GeoWave setup with Mapnik. I would be happy to port that over to whatever format you guys want.

I will work on moving the GeoWave plugin to the git repo that you setup. As I said above, I don't have computer access quite yet at my new work location, and I've got a lot going on at home right now with a new house and baby (excuses, excuses...), but I will try to get the plugin moved over as soon as I can. I apologize for the delay and for this delayed response as well, but I really appreciate your support.

Thanks again,
Whitney

@jwomeara
Copy link
Copy Markdown
Author

jwomeara commented Jul 9, 2015

Hello again,

I'm in the process of getting the geowave plugin checked into the repo that you setup for me. I am going to start by creating some tickets documenting the tasks that lie ahead as I see them. Currently, my time is split between a few things at work, so creating issues should help me to keep track of what needs to be done. I won't pull anything into master until the plugin is in a buildable state.

I apologize for taking so long with this. Life has been a little hectic recently, but things are starting to settle down. Thanks for all of your help so far. We're still very excited to be integrating with you guys!

Take care!
Whitney

@jwomeara
Copy link
Copy Markdown
Author

Hey @flippmoke,

It's me again. I'm not sure if you've noticed, but I've been chugging away on my initial build of the GeoWave plugin under the mapnik/geowave-plugin repo. I've reached the point where I'm ready to start running some travis builds, but unfortunately I need admin access to enable the repo. If you wouldn't mind enabling it for me, I'd be grateful. In the meantime, I'll just fork the repo and setup travis there.

Sorry for going dark for so long. I've had a lot on my plate recently. Regardless, I think the plugin is about 90% complete. I'd like to add a few more test cases, and I need to take a deep dive into documentation as well. Things are coming along nicely though. I'm hopping to wrap things up before the end of the month.

Whitney

@jwomeara
Copy link
Copy Markdown
Author

@flippmoke @springmeyer @chrisbennight

I'm going to go ahead and close this pull request. I wrapped up my initial check in for the GeoWave plugin at the repo that you setup for me. I setup a travis integration test as well, but it needs to be enabled by a mapnik org admin. Once that's turned on, I'll push a small update to get a build going. I was able to verify that everything builds properly in travis by creating a fork of the project, so we should be good to go.

Thanks for the support! It's been an interesting process. I may make an update in the future to add a raster GeoWave datasource as well, so stay tuned for that. Also, let me know if there's anything you'd like to change with the plugin.

Thanks again!

@jwomeara jwomeara closed this Nov 25, 2015
@flippmoke
Copy link
Copy Markdown
Member

@jwomeara It has been an interesting process for us as well! Thank you for working with us through this! We really want to be able to support as many plugins to mapnik as possible and sorry that we made it difficult at times!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants