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
Add GeoIp extension #179
Comments
We have created an issue in Pivotal Tracker to manage this: https://www.pivotaltracker.com/story/show/135465045 The labels on this github issue will be updated when the story is started. |
Hi @mastertinner, the extension seems like a useful PHP extension.
|
Thanks for investigating, @RochesterinNYC! The extension you mentioned is in fact the correct one. I am not sure about The Piwik page where you can set the method of how geolocations should be determined says the following about using GeoIP from PECL:
So it looks like they're recommending the one you suggested. |
It seems like while the extension is free and open source, it requires a database that is not. https://dev.maxmind.com/geoip/geoipupdate/ Not sure that should block us from pulling in the extension, but something to consider since the extension doesn't really do anything without the DB. Perhaps there is some way that we could auto download and install the DB as well. Maybe if a user binds a user provided service with his or her maxmind creds, then we could configure geoipupdate (linked above) to download and keep the db up-to-date (or something along those lines). Another option might be to have the user bind a service (or set an env variable) with the path to the database. Then we could just configure php.ini to point to the correct location for the database. |
I agree with just pulling in the extension. Ascertaining the DB could be done via the The part about auto downloading and installing the DB sounds out of the scope of the buildpack. The last option sounds a bit more in line with what we do with other buildpacks/services like NewRelic. |
This does not fit with the general philosophy of build packs, which is that things should "just work". We should try to make this work without requiring the user to code or do anything beyond binding a service or setting a specific env variable.
Persistence in the container doesn't really matter here. The DB is read-only and just needs to exist so that the geoip extension can look up stuff. The only changes would be if the DB is updated and if this happened, the entire DB would be replaced with a new one. I think if we set the build pack to pull a DB at staging time that's probably sufficient. Users can then restage the app to pull in the latest copy of the DB.
My $0.02. Add a PHP build pack extension that looks for the presence of a variable, say It looks like you can get a direct download URL for the database from MaxMind, so if your CF is on the public Internet you could point directly to MaxMind. If you're not on the public Internet, you could point to a file uploaded with the app or provide a URL to an internal location with the database. Should be easy enough to implement and easy for users to enable. Similar to NewRelic PHP build pack extension, where it looks for the presence of a license key. @mastertinner - Thoughts? How are you obtaining and configuring the DB in other environments? |
@mastertinner is there further information you could give us regarding how you plan to use the GeoIp extension? Specifically in regard to @dmikusa-pivotal's comments about the DB |
@mastertinner thoughts on above comments? |
1 similar comment
@mastertinner thoughts on above comments? |
Sorry that I took so long for me to answer. I would like to use it so that Piwik can look up the geolocations of the IP addresses of visitors of my sites. I agree that persistence is not an issue because the database needs to be read only.
I think that's a great idea. Furthermore, we could set the variable to use the official download URL by default so that most users won't have to configure the variable. In that way, we could keep the mentioned buildpack philosophy about them "just working". The user would just have to enable the extension in I'm not a PHP developer at all so I haven't used this database anywhere else yet... |
Just an fyi, I have some changes to binary-builder which I think will suffice for building the geoip library and optionally packaging the "lite" version of the databases, which are available under a creative commons license (default is not to though). I'll try to share this soon. |
I've submitted a binary-builder PR that adds support for geoip & the dbs. cloudfoundry/binary-builder#24 I have a PR in mind for the build pack too, but first step is to get the extension built and included. |
Awesome! Thanks so much, @dmikusa-pivotal! |
This extension should be available in the latest version of the buildpack. |
Nice! Thx, @sclevine! |
@sclevine, I included it when deploying Piwik and the database is there. However, when I look at Piwik's health check, it tells me
Should we add the database to the folder |
@dmikusa-pivotal thoughts? |
I think you can make this work with a Ex:
The other point to note is that we do not bundle the databases with the build pack. You'll either need to upload those with your app or run the script that we bundle to download them at runtime. Ex:
Unless you have a license file, you probably want to run You can run this by setting @sclevine - I want to update the build pack so that it does this automatically. I just haven't gotten around to putting together a patch. I think the basic idea is that when the I haven't through it through all the way though and there are concerns:
|
I submitted this PR which should make using geoip easier. #204 |
What version of Cloud Foundry are you using?
246
What version of the buildpack you are using?
4.3.23
If you were attempting to accomplish a task, what was it you were attempting to do?
Push Piwik using this tutorial: https://berndsgn.ch/run-piwik-on-cloud-foundry
What did you expect to happen?
I was attempting to install the
geoip
extension of PHP which plays a major role in Piwik. Here's my.bp-config/options.json
for that:What was the actual behavior?
During staging, I get the following message:
Please confirm where necessary:
The text was updated successfully, but these errors were encountered: