geoip2
geoip2 copied to clipboard
Add support for database types other than MaxMind
There are now a number of alternative providers of MMDB databases (including the free Geoacumen database generated from ASN data), but they are unusable with the geoip2 library because it explicitly checks for a MaxMind-specific reader.metadata.DatabaseType
.
This pull request maintains the current behavior while adding the option to specify a custom database type when needed by calling NewXxxReaderType()
instead of NewXxxReader()
. It also accepts Geoacumen-Country
as a valid database type for NewCountryReader()
by default.
Hi, thanks for your interest.
I think, you can just add Geoacumen-Country
to supported types, without other changes. How I can see, Geoacumen
contains only one field in country - iso_code
, this field is supported in the library, but anyway test should be written for this database.
This library has hardcoded available fields, because of that it's not possible to permit user to use any types of database.
Thanks for the response. My thought (and the reason I didn't create tests specifically for Geoacumen) is that there are actually a number of other providers of MMDB databases -- DB-IP is one, IP2Location is another, and there were a number of others that I did not make note of as they didn't fit my specific needs when I was compiling my list. There are also a number of tools available to create your own MMDB databases.
In my (admittedly subjective) experience, they tend to use the same field names as the MaxMind databases, they just don't use a DatabaseType that contains MaxMind's trademarked product names. So the existing DatabaseType checks become an artificial limitation on using third-party databases.
I'm not suggesting you aim to offer formal support for all database types; it would just be nice if we as developers had the option to swap in other databases on a "try it and see" basis without maintaining a fork just for that purpose.
Looks reasonable.
But I think it would be better to use names for functions with suffix like (By|With)Type
and add documentation for this case, when developer takes responsible for correctness of database up to himself.
Sounds good -- I've made the requested changes.
Looks ok, conflicts should be resolved before merge.