search

MontrealCorpusTools / PolyglotDB

Language data store and linguistic query API
MIT License
38 stars 14 forks source link

Update pgdb script for compatibility with updated setup.py and better resource management #190

Closed lxy2304 closed 2 months ago

lxy2304 commented 3 months ago

  1. Refactor for compatibility with entry_points in setup.py: Issue: The script needs to be compatible with entry_points. Proposed Fix: Refactor the script into a function.

  2. Apple Silicon and InfluxDB installation: Issue: Homebrew uses a different folder for InfluxDB on Apple Silicon. Proposed Fix: Add if-entry to check for Apple Silicon and modify folder accordingly. However, it should be noted that:

    • Consider alternatives to Homebrew since it is not installed by default on Macs (eg: download influxDB from https).

    • Homebrew installs the InfluxDB executable in a system-wide location, whereas it should be installed in the environment’s directory when using Conda/venv. That turns into the same issue as mentioned below.

  3. File storage locations in the pgdb script: Issue: Current file storage locations seem problematic during installation, need better resource management. Proposed Fix: Modify the script to identify the user's environment ('CONDA_PREFIX' in os.environ). For example:

    • The config.ini location is currently static (~/.pgdb), which prevents multiple environments from having separate configurations. (If updated, also change CONFIG_DIR in config.py in /polyglotdb to point to the new location.)
msonderegger commented 3 months ago

Hi @lxy2304 -- I checked in with @mmcauliffe about this, and all changes sound good. Notes:

  1. This is related to setup.py stuff noted in your other issue, you could see MFA for a working example.
  2. Yes, it sounds good to move away from Homebrew -- especially since we are assuming polyglotdb is being installed/used in a conda environment, can influxdb just be installed via conda? (and then the documentation would need to be updated, if it assumes Homebrew)
  3. This sounds good -- though he suggests having a new environment variable defined, like PGDB_HOME.

Go ahead and make these changes and put any questions here. Thanks!

lxy2304 commented 2 months ago

Update: The file storage location remained unchanged. As suggested by @mmcauliffe, an environment variable PGDB_HOME is added to handle the config file location.