Conversation
|
Hello, I referenced this pull request on your google groups page here. Please let me know if you have any questions or comments. Thanks, |
271ad09 to
ba1944a
Compare
|
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! |
|
Some initial thoughts:
Would love to help in any way I can. Thanks so much for your pull request 👍 |
|
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? |
|
@flippmoke Re: hesitancy to take on the JVM as a dependency; Re: testing |
ba1944a to
c46424f
Compare
|
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! |
|
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. |
|
@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: 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. |
|
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! |
|
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. |
|
@jwomeara you might look into using @springmeyer do we have good docs for that process yet? |
|
I have everything running now. I'm just running through my python tests Thanks for everything so far! On Tue, May 5, 2015 at 1:14 PM, Blake Thompson notifications@github.com
|
|
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! |
|
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. |
9d521a5 to
f954281
Compare
|
Everything should be good to go now. Please, let me know if you have any questions! |
|
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:
As always, thank you, and if you have any questions or comments for me or the other GeoWave developers, please let us know.
|
|
@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 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. |
|
@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 |
|
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, |
|
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! |
|
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 |
|
@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 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! |
No description provided.