Bookworm-project / BookwormDB

Tools for text tokenization and encoding
MIT License
84 stars 12 forks source link

Ability to change host #92

Closed organisciak closed 8 years ago

organisciak commented 8 years ago

It looks like the ability to change the MySQL host was lost in some refactoring. It would be nice to:

a) Be able to specify a custom JSON with database information. Now that setuptools installs Bookworm into your Python path, any customizations shouldn't require edits to the installed code.

b) Have Bookworm use your global config file. This is done in SQLAPI.py in the following line with the read_default_file arg, but immediately overwritten by the host arg:

            self.db = MySQLdb.connect(host=host,
                              db=database,
                              read_default_file = configuration_file.location,
                              use_unicode='True',
                              charset='utf8')

My current approach is to simply remove the host=host line, keeping the host information in etc/bookworm/my.cnf.

bmschmidt commented 8 years ago

I can't immediately think of any disadvantages to removing the host=host line here; can you? In the short term, just deleting that line should produce good behavior, no?

In the long term, it would be nice to specify multiple hosts (in case a single webserver needs to access bookworms on multiple servers). That could be handled by having a configuration file at something like /etc/bookworm/hosts.yaml that can override the settings for individual hosts. (Another options would be to store multiple ".cnf" files in /etc/bookworm, where the base filename is the bookworm name. That feels a little kludgier, though.)

organisciak commented 8 years ago

I thought maybe there was a reason for you to have 'localhost' explicitly in there? Still, db.connect probably assumes 'localhost' if it's not provided. If removing the hosts argument doesn't break the default case -- 'localhost' database that isn't explicated in my.cnf -- that's what I think is sensible for the codebase.

For multiple hosts, it might be an interim solution to defer to the (eventual) Docker version for the edge cases where you need to access multiple database servers, so the clients bookworms are all in their own containers. I put up a quick repo with my tentative work on that front, the idea would be that you could run something like

docker run -d -p 80:8017 organisciak/bw-standalone

And it would run a daemon with bookworm running on a port of your choosing (80 above). You could have multiple containers mapped to different ports, each with their own config files and not stepping on each others toes. This isn't a 'best' way, but it dulls some of the urgency for solving multi-host support.

On Sun, Jan 31, 2016 at 8:05 PM Benjamin Schmidt notifications@github.com wrote:

I can't immediately think of any disadvantages to removing the host=host line here; can you? In the short term, just deleting that line should produce good behavior, no?

In the long term, it would be nice to specify multiple hosts (in case a single webserver needs to access bookworms on multiple servers). That could be handled by having a configuration file at something like /etc/bookworm/hosts.yaml that can override the settings for individual hosts. (Another options would be to store multiple ".cnf" files in /etc/bookworm, where the base filename is the bookworm name. That feels a little kludgier, though.)

— Reply to this email directly or view it on GitHub https://github.com/Bookworm-project/BookwormDB/issues/92#issuecomment-177730952 .

organisciak commented 8 years ago

Done with 7988f0d8b26f. There were two places where localhost was hardcoded, which is removed now. Reads explicitly defined host properly from /etc/bookworm/my.cnf in my test environment.