leaflet-layer-overpass icon indicating copy to clipboard operation
leaflet-layer-overpass copied to clipboard

Add an option to choose to make overpass requests per bbox or per tiles

Open GuillaumeAmat opened this issue 9 years ago • 10 comments

GuillaumeAmat avatar Jul 29 '15 04:07 GuillaumeAmat

I don't completely understand what this changes. Can you explain a little how the old version worked and what you enabled in your new version? Especially what this could be used for? It looks interesting.

k-nut avatar Jul 29 '15 07:07 k-nut

The purpose of this Leaflet module is to create Lealfet layers with Overpass requests associated to them. Then it launches automatically the request when we move/pan the map.

The old behavior was to launch one request per tile on the map. A tile is sort of a piece of the map puzzle. So, 10 requests were launched per layer each time we dragged the map (not totally true but it's the idea).

My modification adds an option to stick to this behavior or disable it by launching only one request per layer and per move/pan.

GuillaumeAmat avatar Jul 29 '15 07:07 GuillaumeAmat

Ah sounds good. I always wanted to also make a change to limit the requests but never found the time to do so. :+1:

k-nut avatar Jul 29 '15 08:07 k-nut

A few remarks:

  • it would be great to also have a [timeout:...] to avoid HTTP 429 errors
  • when panning, the currently active query should be killed by issuing kill_my_queries towards Overpass API

mmd-osm avatar Aug 18 '15 08:08 mmd-osm

Do you think maybe you could add those changes? :smirk: They sound like a very good idea.

k-nut avatar Aug 18 '15 08:08 k-nut

First of all thanks for your work @GuillaumeAmat I want to merge the first 2 commits see #30 (later ones are fork specific but I like the features).

Now I would like to point out why I used tile like requests

  • it's easy cachable
  • no requests on zooming in or while moving the map in an already loaded area

Nevertheless I agree that the bbox approach reduces the request count for (most?) use cases.

kartenkarsten avatar Aug 18 '15 20:08 kartenkarsten

@kartenkarsten I totally agree with your two points and knew that, that's why I added a « switch » option to activate one solution or the other.

In my developments, I encountered many errors due to the number of requests from the Overpass server. Those errors motivated the change. I think I will add @mmd-osm's proposal in my fork by adding an option to use kill_my_queries if a request is already awaiting a response (to avoid multiple requests on pan/zoom).

Also, I need live data and can't cache the Overpass returned ones.

I don't know if I already told you but thank you too for your work ;)

GuillaumeAmat avatar Aug 20 '15 15:08 GuillaumeAmat

@mmd-osm instead of kill_my_queries, wouldn't simply calling abort() on the running request also work to cancel it on the server (at least that's what I do with Achavi)?

nrenner avatar Aug 20 '15 15:08 nrenner

@nrenner : looking at how this is implemented in overpass turbo, we even see both variants there: first the abort() call then kill_my_queries. The latter will simply evoke a shell script on the Overpass server to kill the interpreter process on OS level (sadly, this is not working on overpass-api.de at the moment, but will hopefully get fixed in the future - see https://github.com/drolbr/Overpass-API/issues/214).

I don't know exactly how reliably abort() works with Apache and CGI processes, need to investigate this part.

Edit: I just deployed achavi on the test box, adjusted the URLs to point to the test server instead, ran a query and immediately pressed cancel. The interpreter process keeps on running on the server, even after the abort() call. kill_my_query indicates, that there's still a query running on the server, which is being killed now. Maybe this is something specific in the way Apache runs CGI processes?

mmd-osm avatar Aug 20 '15 20:08 mmd-osm

@mmd-osm thanks for your answer and investigation.

Maybe this is something specific in the way Apache runs CGI processes?

Can't help you with that, I'm afraid.

nrenner avatar Aug 26 '15 17:08 nrenner