digitalpalidictionary / dpd-db

12 stars 7 forks source link

bash/makedict.sh errors on the first run #19

Closed gambhiro closed 6 months ago

gambhiro commented 7 months ago

Related to #18

Starting in a fresh repo, running bash/makedict.sh fails with the error:

config setting updated: 'dictionary, link_url' is 'https://www.thebuddhaswords.net/'
create inflection templates
Database file doesn't exist: /home/gambhiro/prods/apps/simsapa-project/bootstrap-assets-resources/dpd-db/dpd.db

build_db.sh and makedict.sh seems to run many of the initial calculations.

Is it necessary to run build_db.sh before makedict.sh?

gambhiro commented 7 months ago

This seems to be an error.

The two scripts share many of the same steps, such as running sandhi_splitter.py.

If makedict.sh expects an empty db to be present, it should create it.

If there are preparation steps which both scripts depend on, I recommend extracting those steps to another script file which both build_db.sh and makedict.sh can run at the beginning.

Devamitta commented 6 months ago

As I understand the sequence:

poetry run bash bash/initial_setup_run_once.sh

poetry run bash bash/build_db.sh

poetry run bash bash/make_dict.sh

gambhiro commented 6 months ago

The point was that makedict.sh seems to depend on build_db.sh to run without errors, but makedict.sh starts by repeating several steps which are already included in build_db.sh

So if makedict.sh depends on every step in build_db.sh, it should run build_db.sh directly, and remove the duplicate steps.

Or if there are a series of "preparation" steps which both depend on, those steps should be separated to another script, which build_db.sh and makedict.sh can run directly, again avoiding duplicate steps.

Devamitta commented 6 months ago

Now, with the new commit, their overlap is in the generating_components.sh file, and they all run it. However, initially, you must execute either build_db.sh or build_and_make_all.sh for the first time. In the newly copied repository, you cannot simply run makedict.sh.

gambhiro commented 6 months ago

👍 Thanks, I think that clarifies it somewhat.

you cannot simply run makedict.sh

If the expected behaviour is that it should not be used on it's own, then it should check for the necessary conditions (e.g. database being present) and print a useful message with some minimal instructions.

Database file doesn't exist: ... could be a user error, system error, permissions error, etc.

One other thing about scripts in scripts in a venv:

Either the script should activate the venv at the start, and no subsequent commands should use poetry run ..., or none of them should, and the venv is expected to be activated by the user (poetry shell).

venv in a venv is not a good idea and makes debugging confusing. It would be best if only top-level scripts were started by poetry run ... , and sub-tasks will be already executing in that venv.

Devamitta commented 6 months ago

Ok, thanks. Got rid of poetry reference inside the bash.. And added check for db file.