Skip to content

Add Maxmind support (using Maxmind Web Services) #227

Merged
merged 5 commits into from Dec 21, 2012

5 participants

@gonzoyumo

Hi,

here is Maxmind support that I added 7 months ago on my fork.

I'm sorry to forgot to do a PR but it has the good point that this service has been used for 7 months now (on production servers)

Any comment is welcome and if you feel it okay, it would be great to have it merged into main repo.

Thanks.

@kevinwmerritt

This is great!. Thanks. I will need to try maxmind because free geo ip is not very accurate.

@alexreisner
Owner

Thanks for this. Just wanted to let you know that I will merge it, but need to re-think API key configuration first. Will probably be included in Geocoder 1.1.3.

@dpehrson

This would be most excellent, I have a paid MaxMind GeoIP City database and would love to replace geoip+geocoder with just geocoder and a unified API.

@gonzoyumo

@alexreisner, good news !
@dpehrson, sorry but this pull request doesn't add support for downloaded database file. It only works with Maxmind GeoIP Web Services (http://www.maxmind.com/app/web_services).
I updated the title to be more explicit.

In fact, there are different levels of web services, which provides different amount of data. I did it for the "GeoIP City/ISP/Organization Web Service", additional stuff may be needed to have it fully functional for Services that provide more data.

@mltsy
mltsy commented Jun 12, 2012

I would totally use this! :)

@alexreisner alexreisner merged commit 61a4ddb into alexreisner:master Dec 21, 2012
@alexreisner
Owner

Merged. Thanks!!

@alexreisner
Owner

Could someone who uses MaxMind do me a huge favor and test this? I've asked MaxMind for some free requests but I'm not sure if/when they will grant them, and this is the last item blocking the release of gem version 1.1.6. To try it, just put this in your Gemfile:

gem 'geocoder', :git => 'git://github.com/alexreisner/geocoder.git'
@gonzoyumo

Hi Alex, I'm happy to see this is going forward!
It has been a while since I've used geocoder as I'm not anymore in charge of the project where I used it. Anyway I was too curious so I couldn't resist to launch a console :)

After bundling with master, I first got an exception when launching console:

undefined method `ip_lookup_api_key=' for Geocoder::Configuration:Class (NoMethodError)

So I took a look at the readme and searched for the other name you may have used but I saw you just removed that param. This isn't a good idea to me because of the following case :

Once you've configured your Maxmind API key in api_key attribute, you won't be able to use a text address lookup service (not ip) because it will use the same api_key.

IE:

> Geocoder.configure(:lookup => :google, :ip_lookup => :maxmind, :api_key => "my_maxmind_key")
=> {:timeout=>3, :lookup=>:google, :ip_lookup=>:maxmind, :language=>:en, :http_headers=>{}, :use_https=>false, :http_proxy=>nil, :https_proxy=>nil, :api_key=>"my_maxmind_key", :cache=>nil, :cache_prefix=>"geocoder:", :always_raise=>[], :units=>:mi, :distances=>:linear} 
> Geocoder.search("some.secret.ip.address")
=> [#<Geocoder::Result::Maxmind:0x007ff68ad3f468 @data=["US", "TX", "Houston", "77210", "29.xxxxx", "-95.xxxxxx", "618", "713", "Shell Information Technology International", "Shell Information Technology International"], @cache_hit=nil>] 
> Geocoder.search("Houston")
Google Geocoding API error: request denied.
=> []

It seems that Google doesn't like my Maxmind api_key :(

Or did I miss something?

@alexreisner
Owner

@gonzoyumo thanks so much. Searching for an IP address should make it use MaxMind instead of Google:

Geocoder.search("74.200.247.59")

Let me know how that goes.

As for the configuration--you're absolutely right. That's why I didn't implement this sooner. I actually reworked the entire configuration syntax so you can specify keys, timeout, language, cache, etc for any number of APIs simultaneously. The intended use is something like this:

Geocoder.configure(
  :lookup => :yahoo,
  :ip_lookup => :maxmind,
  :yahoo => {:api_key => "...", :cache => Redis.new},
  :maxmind => {:api_key => "..."}
)

Any options specified at the top level will be defaults for all lookups. Make sense? This will all be added to the README and released, hopefully in the next couple of days.

@gonzoyumo

ahhh my bad, sorry :/ I should have looked at the code

Using correct configuration syntax it works like a charm 👍

Just another remark: config/initializers/geocoder.rb sample file in the readme is ending with end instead of a parenthesis

Thanks you again for that great gem, I will definitely use it again when I'll need lookup service :)

@alexreisner
Owner

Great. Thanks again. Really helpful.

One other thing, since it's about code you wrote: any thoughts on d97f399? Which documentation is your comment referring to? Ruby's CSV library? Would be great if we could get a sample response from MaxMind with some extended characters to use in a test.

@gonzoyumo

This was from Maxmind website, it seems that it has changed since but the documentation is still pretty clear about this:

All strings are returned in the ISO-8859-1 encoding. This encoding is also referred to as latin1.

you can find it in "ouput" section there: http://dev.maxmind.com/geoip/web-services

looking for a sample response...

@gonzoyumo

Here is a sample result string obtained with raw_data.inspect :

"FR,A3,Ch\xE2lo-saint-mars,,48.427601,2.065400,0,0,\"Free SAS\",\"Free SAS\""

As you may see, maxmind documentation provide a ruby 1.9 example and they use:

response.body.encode('utf-8', 'iso-8859-1').parse_csv

.encode('utf-8', 'iso-8859-1').parse_csv works well when applied to raw_data string, so you may use it I think

I don't know about ruby 1.8 :/

@alexreisner
Owner

Thanks for the sample data! Unfortunately neither the current solution nor theirs seems to work!

I wonder if Geocoder should be doing any conversion at all. Granted, among us westerners UTF-8 seems to be the current encoding of choice, but some people might want to convert to something else (eg: UTF-16 or -32), in which case this would be some wasted CPU cycles.

I do think it's worthwhile, though. Most of the other supported services (including Google and Bing) return UTF-8 and perhaps more care should be given to ensure that all results are UTF-8 encoded.

However, I am not going to do the conversion with Ruby 1.8.x. Unless I'm mistaken, every method of changing character encoding in Ruby 1.8.x requires some external system dependency (like iconv), which is too much complexity for Geocoder. I don't like this inconsistency between Ruby versions, and will make a note of it, but I'm not going to introduce a large dependency for what is sort of a minor issue.

@alexreisner
Owner

Also, what's the reasoning behind the substrings in Geocoder::Result::Maxmind#isp and #organization? Those values appear to be getting truncated.

@gonzoyumo

hmm, I didn't catch that. To be honest I don't remember why I did such a thing (I should have written a comment), but I think it was mandatory to get clean values ^^

isp and organization are the only attribute that are wrapped with double quotes in the result string. I suppose there was something weird with this and I was forced to manually trim them.

It seems that it's not the case anymore so it should be dropped as they are indeed cutting values now.

@gonzoyumo

Ah, one more thing about my code: I only focused on supporting the "City/ISP/Org" service (the one we were using).

But Maxmind also provides other services which contain less or more attributes, making the current code failing for them.

This is due to the fact that Maxmind response length is different for each services, see http://dev.maxmind.com/geoip/web-services#Output_field_order-3

So it's worth warning user about this or implementing other services for full support.

thanks

@alexreisner
Owner

Thanks for pointing out the different response types. Sounds like we need to support all four types. Here's what I'm going to do:

  1. Fix the isp/organization trimming.
  2. Implement configuration/support for the four MaxMind services/response types.
  3. Remove string encoding conversion.

The string encoding is too complicated to implement correctly/consistently before the 1.1.6 release (which I'd like to do today or tomorrow). I'll create an issue about handling encoding from all APIs consistently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.