protomaps / OSMExpress

Fast database file format for OpenStreetMap
BSD 2-Clause "Simplified" License
229 stars 19 forks source link

Fixup and document augmented diff example #26

Closed CloudNiner closed 3 years ago

CloudNiner commented 3 years ago

This PR fixes a few issues I found while integrating and productionizing this example for https://github.com/azavea/onramp.

bdon commented 3 years ago

More of a meta-comment @CloudNiner - I wonder if it makes sense to move the code in examples/augmented_diff and osmx-update into the osmx python module. That way a client like onramp can simply import these without having to duplicate the code. It would also allow for some separation of concerns; parts related to compressing and uploading diffs to S3 for example can remain in in the client code, while the "meat" lives inside the pip-installable python module

CloudNiner commented 3 years ago

I wonder if it makes sense to move the code in examples/augmented_diff and osmx-update into the osmx python module.

I was thinking about this a bit as well. In the case of onramp, we need to inject the augmented diff operation in between pulling the osc file and osmx update. If you were to pull osmx-update into the osmx module, we'd need some hook that allows us to run arbitrary code in that loop. I'm not sure how many other use cases there are for that, but it's feasible. It's also the case that many different environments have many different constraints, so it's unclear to me if osmx-update is a thing that should be elevated to part of the library. That would lead to additional burden in order to support and document osmx-update as the canonical way to update an osmx db in deployed environments. But for us, it was a great starting point to be able to drop osmx-update into our own setup.

I'm not so sure about pulling augmented_diff into osmx. It's really more a "thing you can do" with the osmx bindings than it is a core part of what osmexpress is all about. It also requires supporting infrastructure to make work, in that you need an osmx instance and a replication server to hit. But it does serve as a really good non-trivial example, the python augmented diff computation is good enough that we've abandoned the C++ version for the time being.

bdon commented 3 years ago

Agreed on all points re: moving the code into python module. It would be very appreciated if you can continue to merge in any relevant fixes to the examples or osmx-update!