search

elixir-geolix / geolix

IP information lookup provider
Apache License 2.0
190 stars 18 forks source link

Update the README for installing with Phoenix #9

Closed acconrad closed 8 years ago

acconrad commented 8 years ago

After dealing with some issues setting this up on Phoenix, I wanted to update the documentation once I got it working.

acconrad commented 8 years ago

@mneudert any ideas on how to process an application/octet-stream? I can request an .mmdb/mmdb.gz file just fine from :httpc.request and I did so right about here, but it's just a List of Numbers (e.g. [101, 29, 113]) so I'm not sure how to turn that into a File so it can be properly digested and read.

mneudert commented 8 years ago

I think we are on the right track here but should somewhat condense the now 2 blocks of configuration.

If I am not missing anything here then Phoenix should not need any specific handling. It might be the "suggested path to include for release deployment", but dabbing into deployment issues here is probably too much.

So that basically leaves us with three generic ways to configure the path independent of what else runs on the system:

# Path.join/2 should be the same as your :filename.join/2 (just pure Elixir)
path_abs  = "/absolute/path/to/database.mmdb"
path_rel  = Path.join(__DIR__, "../data/database.mmdb")
path_priv = Path.join(:code.priv_dir(), "static/database.mmdb")

This could be included into (or better, replace) the already existing configuration section.

Also might help with also including a fourth configuration for remote files once that is ready :+1:

mneudert commented 8 years ago

The data you are seeing is probably a char_list and a simple iodata_to_binary/1 on the result should work here.

If you move the File.exists?/1 from there to Geolix.Reader.read_database/1 you should easily be able to replace the FIle.read!/1 call with a private method. This method should return just the binary contents of the database, either from a file or the request. That should eliminate any needs for temporary files and save some headaches.

mneudert commented 8 years ago

Hey there, do you have any updates on this for me?

acconrad commented 8 years ago

Hey @mneudert this totally went under my radar - wondering if we should update the docs for now, and a separate PR for allowing remote files. What do you think? I can probably work on it sometime this weekend.

mneudert commented 8 years ago

The remote file feature should definitely be a separate PR. Just to have the feature split apart from the documentation update.

mneudert commented 8 years ago

ping @acconrad :smiley:

acconrad commented 8 years ago

FINALLY got to this, see #10 for the code! Thanks for being so patient, and sorry for going ghost for so long!

mneudert commented 8 years ago

Will take a look at that!

But back to the topic of this PR. I pushed a different version of your changes to the branch doc.

Some time ago I became quite aware of problems with anything "dynamic" in configuration files. Especially when it comes to releases. The problem is: dynamic stuff breaks more often than not.

A viable solution was to just add a hook to allow the usage of environment variables.

For this problem here it would mean that whatever gets documented is still errorprone because of the compile time evaluation of everything in the configuration file. Even :code.priv_dir(:some_application) returns an absolute path. And that one is probably far different from the production environment (stuff like "/tmp/project/_build/test/lib/geolix/priv").

So I would just keep it at making the absolute path configuration obvious and include a notice about non-absolute paths.

Would that be ok for you or do you have a better idea?

acconrad commented 8 years ago

Sounds good to me, I just want to make sure if someone is using this with Phoenix, that it is intuitive in terms of how to set up, but hopefully the remote path PR will mitigate some of that issue, because you can simply supply a URL (as most would on a Phoenix app) to get up and running quickly.

~Adam @adam_conrad http://twitter.com/adam_conrad http://www.adamconrad.net Find me on LinkedIn http://www.linkedin.com/in/acconrad

On Wed, Mar 30, 2016 at 2:55 PM, Marc Neudert notifications@github.com wrote:

Will take a look at that!

But back to the topic of this PR. I pushed a different version https://github.com/mneudert/geolix/tree/doc#configuration of your changes to the branch doc https://github.com/mneudert/geolix/tree/doc.

Some time ago I became quite aware of problems with anything "dynamic" in configuration files. Especially when it comes to releases. The problem is: dynamic stuff breaks more often than not.

A viable solution was to just add a hook to allow the usage of environment variables.

For this problem here it would mean that whatever gets documented is still errorprone because of the compile time evaluation of everything in the configuration file. Even :code.priv_dir(:some_application) returns an absolute path. And that one is probably far different from the production environment (stuff like "/tmp/project/_build/test/lib/geolix/priv").

So I would just keep it at making the absolute path configuration obvious and include a notice about non-absolute paths.

Would that be ok for you or do you have a better idea?

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/mneudert/geolix/pull/9#issuecomment-203579350