✨ Docker configuration for nominatim#327
✨ Docker configuration for nominatim#327madmath03 wants to merge 5 commits intoProject-OSRM:gh-pagesfrom
Conversation
Signed-off-by: mathieu.brunot <mathieu.brunot@monogramm.io>
akashihi
left a comment
akashihi
left a comment
There was a problem hiding this comment.
I'm not a frontend expert, so it would be nice if someone experienced will review that too
|
Im interested in this commit. It solves one problem for me. How can I help to make it in main code base? |
The shortest path (pun intended!) to getting this merged is a rebase onto latest main branch. 😉 |
There was a problem hiding this comment.
Pull request overview
Adds Docker-focused configurability for the Nominatim geocoding endpoint, allowing deployments to point the frontend’s address lookup at a custom Nominatim instance (addressing the “where is NOMINATIM config?” requests).
Changes:
- Introduces a
nominatim.urloption insrc/leaflet_options.jsand wires it intoleaflet-control-geocoderinitialization. - Adds
NOMINATIM_URLenv var support to the build-time replacement script and Docker image defaults. - Documents the new Docker environment variable in the README.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/leaflet_options.js | Adds a configurable Nominatim URL to frontend options. |
| src/index.js | Passes configured Nominatim URL into Geocoder.nominatim(...). |
| scripts/replace.js | Supports NOMINATIM_URL env var for build-time config injection. |
| README.md | Documents NOMINATIM_URL usage for Docker runs. |
| docker/Dockerfile | Sets default NOMINATIM_URL in the Docker image. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| You can change the backend by using `-e OSRM_BACKEND='http://localhost:5001'` in the `docker run` command. | ||
|
|
||
| Per default address requests are made against the Nominatim demo at `https://nominatim.openstreetmap.org`. | ||
| You can change the Nominmatim instance by using `-e NOMINATIM_URL='http://localhost:8080'` in the `docker run` command. |
There was a problem hiding this comment.
Typo in documentation: “Nominmatim” should be “Nominatim” (environment variable description).
| You can change the Nominmatim instance by using `-e NOMINATIM_URL='http://localhost:8080'` in the `docker run` command. | |
| You can change the Nominatim instance by using `-e NOMINATIM_URL='http://localhost:8080'` in the `docker run` command. |
| const NOMINATIM = process.env.NOMINATIM_URL || 'https://nominatim.openstreetmap.org' | ||
| const DEFAULT_LAYER = process.env.OSRM_DEFAULT_LAYER || 'streets' | ||
|
|
||
| // Edit Leaflet Options | ||
| if (NOMINATIM) options = options.replace(/\/\/nominatim\.openstreetmap\.org/, NOMINATIM) |
There was a problem hiding this comment.
NOMINATIM_URL values that already include a trailing slash will produce a double-slash in leaflet_options.js (because the template keeps its existing /). Consider normalizing NOMINATIM (trim trailing slashes) or adjusting the regex to replace the full default URL including the optional trailing / and then append exactly one /.
| var plan = new ReversablePlan([], { | ||
| geocoder: Geocoder.nominatim(), | ||
| geocoder: Geocoder.nominatim({serviceUrl:leafletOptions.nominatim.url}), | ||
| routeWhileDragging: true, |
There was a problem hiding this comment.
index.html loads the committed bundle.js, but bundle.js in the repo still uses the old r.nominatim() call and doesn’t include leafletOptions.nominatim. This change won’t take effect for the static build unless npm run build is run and the regenerated bundle.js is committed (or the build/publish pipeline is updated to generate it).
✨ Docker configuration for nominatim
Closes #313 and closes #314 and closes #283.