Skip to content

Fix dependencies issues shapely/MVT and free localost from osmose.openstreetmap.fr #540

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

flacombe
Copy link
Contributor

@flacombe flacombe commented Jul 6, 2025

Fix two issues preventing local map to run when testing or dev:

  • Python's mapbox-vector-tile depends on shapely 2.0.4 but >=2.1.0 already removed deprecated methods, so limiting required version prevent breaking changes
  • Web map was still linked to osmose.openstreetmap.fr to get maplibre's sprites. It is proposed to expose them directly in uvicorn app and allow localhost:8080 to query localhost:20009 to get them

I recommend to give it a try on public infrastructure. I didn't know how marker-gl-sprite is exposed today as there is no mention in uvicorn app nor in apache-site files.
This PR intends to establish a common way to serve those files both in dev and in prod.

@@ -10,7 +10,7 @@ module.exports = (env, argv) => {
},
plugins: [
new SpritezeroWebpackPlugin({
source: '../web_api/static/images/**/*.svg',
source: 'images/**/*.svg',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May you need this, because you de not run it from the right path ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I run this from the docker-compose, in the osmose-frontend/docker directory

docker-compose -f docker-compose.yml -f docker-compose-test.yml up

And it didn't work unless I make the path relative to the config file (which is already in web_api/static)

osmose.py Outdated
Comment on lines 79 to 88
@app.get("/assets/marker-gl-sprite.json")
def sprite_png():
return Response(open("web_api/public/assets/marker-gl-sprite.json", "rb").read())


@app.get("/assets/marker-gl-sprite.png")
def sprite_png():
return Response(open("web_api/public/assets/marker-gl-sprite.png", "rb").read())


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May just implement it in a generic way in the catch_all ?

Copy link
Contributor Author

@flacombe flacombe Jul 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The catch_all is rooted in web/public and assets are in web_api.

Routing appropriate resources between web/public and web_api in catch_all is equivalent to define dedicated routes

I wondered how these resources are served today and didn't found out

osmose.py Outdated
@@ -11,6 +12,23 @@

app = FastAPI()

origins = [
Copy link
Contributor

@frodrigo frodrigo Jul 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CORS on non public API are on purpose.

I'm OK for CORS as middleware, but only for API. Host can be *.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may find more appropriate strategy for origins (I choose the default one).
But I was unable to get https://osmose.openstreetmap.fr/assets/marker-gl-sprite.png from localhost:8080 when opening the map in my browser.

localhost should rely on localhost resources only and we may adjust appropriate origins depending on env prod/dev

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed @frodrigo, I didn't understand properly the nesting with FastAPI.

I've specified more appropriate CORS for web assets only, and preserving the existing CORS for everything else.

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.

2 participants