OSM Nominatim does not work

Dont know what it is for, but thanks. I have applied the patch.

I am using osm nominatim. Does it have an api or I can just use it without api?

I've just applied the patch against latest DEV release and I'm still getting 'No results for geocoding' when OSM is selected. Same nodes work fine when Google Geocoding API is used.

@robcarr check your query against https://nominatim.openstreetmap.org

@denix Tested various addresses directly at https://nominatim.openstreetmap.org as suggested: they work fine. But when used from Drupal I still get 'No results for geocoding'... whereas Google geocodes fine

?

We ran into this issue when we were starting to use this on www.drupal.org.

#6 works well, although this won’t do what it is trying to do:

+  // Ensure an absolute maximum of 1 request per second.
+  // @see https://operations.osmfoundation.org/policies/nominatim/
+  sleep(1);

That just slows down the current request. If another request is made, it will be in another process, unless you are doing bulk geocoding. There should be a way to use locking to not slow down people, and enforce the limit across all processes.

Thanks @anrikun for putting all this together. Following @drumm comment, I came up with this version that include a few lines of documentation, a delay system to limit execution to 1sec for batch executions, the use of a valid email according to policy of use and few extra messages so we now know what's happening with our calls. I do really hope that the developers will release a new stable soon!

Hi @drumm, I was looking at using lock_acquire to grant lock to the function, but I am not sure that it is the right solution.
I did some tests, but I could not see any real difference.
Do you have any experience with it? or a sleep is enough?
There is also the risk of lock for a very long time if you hit a timeout for example.

This looks like a good start, and an improvement over what was there. The delay won’t happen until a second request after the first.

The delay is still hard-coded to 1s, so if code is doing something like bulk geocoding, that could be excessive. For example:

1. The first geocoding request completes, this starts the 1s clock of how long we should wait.
2. Other regular Drupal processing happens, like saving the result or preparing the next data to geocode. This will take a non-zero amount of time.
3. On the second geocoding request, we really only need to wait 1s from when the first completed, not 1s from now.

Static variables are only static within one PHP process - so if two requests come in simultaneously, or bulk processing is happening in parallel, the 1s delay won't be enforced.

Using lock_acquire() will prevent simultaneous requests, and I think is likely to be part of the solution here. I don’t see functionality in the locking system to wait 1s after the last lock has been released.

I expect the time of the last geocoding will have to be stored somewhere, like variable_set(…, microtime())/variable_get(). Frequently-changing variables aren’t great for the variables cache, but this is probably fine for the average site that just geocodes an address or two when content is changed.

The cache system could be abused, setting a 1s expiration. Cache clears are rare enough, risking losing the delay if the cache clears is probably fine. But the cache expiration time only has a 1s resolution, so it might not be ideal.

Otherwise, I’m not aware of an ideal system in Drupal to enforce with without drawbacks. I recommend just picking one, and maybe filing a followup issue for a perfect solution. The drawbacks are only if your site is doing enough geocoding to potentially bump into the rate limit, if someone has that use case, they can pick up the followup issue.

@drumm I think that the issue of lock should be set at the level of geocoder, and not left to the single plugins, what do you think? If you agree then we can set this issue as "reviewed & tested" and move the discussion about the lock to a separated issue.