Skip to content
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

Allow Geoip to auto-update geolocation databases ending with 'mmdb.gz' #11081

Merged
merged 1 commit into from Apr 26, 2017

Conversation

mattab
Copy link
Member

@mattab mattab commented Dec 25, 2016

from #11049

@mattab mattab added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Dec 25, 2016
@mattab mattab added this to the 3.0.1 milestone Dec 25, 2016
@sgiehl
Copy link
Member

sgiehl commented Dec 27, 2016

LGTM, but I'm unsure if that really solves the issue correct.
As far as I've understood, the problem is that Piwik is unable to auto update a geoip2 database, which is not support by Piwik, but a third party plugin. maybe it would be better to create hooks, that make it possible for the third party plugin to extend those method?

@slawa-dev
Copy link

Download completes now, but Piwik does more checks to the file which fail. See: #11049 (comment)

@mattab
Copy link
Member Author

mattab commented Dec 27, 2016

maybe it would be better to create hooks, that make it possible for the third party plugin to extend those method?

I think it's supposed to already work this way. the code triggering the error is: https://github.com/piwik/piwik/blob/comment_match_implementation/plugins/UserCountry/GeoIPAutoUpdater.php#L506-511

as you can see it's calling $provider->getLocation(array('ip' => GeoIp::TEST_IP)); which should use the GeoIP2 provider here. So i'm not sure what the error is... @sgiehl or @S1awa maybe you'd like to take a look...

@slawa-dev
Copy link

Might be related to the problem with the GeoIP2 Plugin: #11034

I'm not familiar with the Piwik code base. I need more time to dive in the code which I currently do not have. End of January I can contribute more than reporting issues.

@mattab
Copy link
Member Author

mattab commented Apr 26, 2017

Hi @S1awa

Do you maybe have some time to take a look at this PR and making the geoip auto updater work?

@slawa-dev
Copy link

@mattab The plugin dev has not fixed his plugin yet. My guess is this PR would pass if the Plugin would work.
diabl0/piwik-geoip2#3

@mattab
Copy link
Member Author

mattab commented Apr 26, 2017

@S1awa Sounds good! Maybe you have a bit of time to help the plugin developer make the plugin work?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants