-
Notifications
You must be signed in to change notification settings - Fork 820
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Flex version of the osm2pgsql configuration #4978
base: master
Are you sure you want to change the base?
Conversation
I have
I have not touched the stuff in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have finished a first reading over the code, not tested it yet.
It looks fairly clean and strait away to understand. Some questions and comments inline.
Regarding the stuff in scripts/lua
- that was introduced in #2128 but it was never properly documented how style developers are to use it, the readme essentially discourages its use. As indicated in the past i am very much in favor of introducing more automated testing to support style development and detecting unintentional changes in style behavior. But that would primarily be important for the rendering stage and less for the data import (see https://imagico.de/blog/en/systematic-testing-in-map-design/). And i don't really see this realistically being implemented through volunteer work.
For the data import stage testing - with the move to flex backend including customized processing not only of tags but also of geometries, proper testing would need to include that, i.e. it would need to assert match between input OSM data and output database content - and i don't think this is possible from within LUA.
-- --------------------------------------------------------------------------- | ||
|
||
-- Needed for use with the Themepark framework | ||
local themepark = ... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am fine with leaving this in for now.
My reasoning is that - while as is there is no substantial benefit from using that - we will need to evaluate that based on the things we ultimately want to do with the flex backend (like the relations and boundary processing).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, there is a lot of benefit there immediately for that. One of the most often requested features we get is: Can I have one database with both the data for rendering a map and also Nominatim for geocoding in it. This is possible with the flex output, you "just" need both configuations side by side. And the way to do that is using the Themepark framework which will "call" both configurations. The Nominatim config currently is not Themepark-ready, so we are not quite there yet, but with the Themepark support for OSM Carto you can already have the tables for OSM Carto and for the Shortbread scheme for vector tiles side by side in the same database.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Frankly, this is not really our concern here. We have the goal of adaptability and ease of use, but for the database this mainly means we try to ensure that the database schema can be customized easily and that other styles can easily co-use our database. Ensuring that sysadmins have it easier to deploy complex setups with different database schemas together is not our goal.
As said: I am fine with keeping the code for both using and not using themepark in for now and defer the actual decision for when we have a better basis for that. I am not fine with deciding to rely on a framework, which - by its own presentation - is in beta test.
This commit contains one file with an osm2pgsql configuration for the flex output that can be used instead of the old configuration for the pgsql output. It replaces the openstreetmap-carto.style and openstreetmap-carto.lua files. The configuration is nearly 100% compatible to the old one. The database layout will be exactly the same with just very little changes. The id columns (`osm_id`) and geometry columns (`way`) on all tables will get the NOT NULL flag when using the flex output. These have always been NOT NULL in practice anyway. The content of the database will be the same with only minor irrelevant differences. Run like this: osm2pgsql -O flex --style openstreetmap-carto-flex.lua -d gis ~/path/to/data.osm.pbf
This commit does the actual switch to the new flex config. It removes the old config files and updates the documentation and various scripts.
They don't work any more with the new flex Lua output. And they are have not been maintained anyway.
I have rebased the PR to master and added a commit removing the outdated Lua test scripts. Is there anything else I can do to get this merged? |
For your information: The osm2pgsql version 2 released yesterday marks the pgsql output as deprecated. |
This commit contains one file with an osm2pgsql configuration for the flex output that can be used instead of the old configuration for the pgsql output. It replaces the openstreetmap-carto.style and openstreetmap-carto.lua files.
The configuration is nearly 100% compatible to the old one.
The database layout will be exactly the same with just very little changes. The id columns (
osm_id
) and geometry columns (way
) on all tables will get the NOT NULL flag when using the flex output. These have always been NOT NULL in practice anyway.The content of the database will be the same with only minor irrelevant differences.
Run like this:
osm2pgsql -O flex --style openstreetmap-carto-flex.lua -d gis ~/path/to/data.osm.pbf
See #4977