lucasdiedrich / ojs

Open Journal Systems (OJS) is a journal management and publishing system.
GNU General Public License v3.0
19 stars 64 forks source link

Feature request : re-create automatically the database if there are no tables #18

Open Potomac54 opened 5 years ago

Potomac54 commented 5 years ago

Hello,

Currently we use an environment variable for forcing the installation : OJS_CLI_INSTALL, when this environment variable is set to 1 then ojs-cli-install is called,

but in fact we can fully automate this process without using an environment variable, by adding some logic in ojs-pre-start script :

lucasdiedrich commented 5 years ago

Hi @Potomac54,

I don't think it should be the containers reponsability to verify if the database exists, we're adding some logic to a simple bootstraping script. Be we can analyse that, if not to complicated using mysql client over bashscript. If you have time, please fell free to make an PR.

What do you think @marcbria ?

Thanks for all @Potomac54.

Potomac commented 5 years ago

Hello,

I am a beginner in docker, but I think mysql client (mysql binary) is not available inside the OJS image,

as a workaround we can use php script from ojs-pre-start script, with 4 arguments (db host, db login, db password, db name) :

php /usr/local/bin/check_db.php ${OJS_DB_HOST} ${OJS_DB_USER} ${OJS_DB_PASSWORD} ${OJS_DB_NAME}

the php script will echo the number of tables found in database, we can put the echo result in a bash variable, like $num_tables :

if $num_tables = 0 then run the curl command for tables creation (ojs-cli-install), if $num_tables > 0 then assume that database is already created, and if config.inc.php is present then no need to run curl command

a more robust check of the database can be done (a valid OJS database has 112 or 119 tables), but I chose a simple approach ( zero tables = no database installed),

you can still keep an environment variable for forcing the reinstallation of OJS, something like OJS_FORCE_INSTALL=1,

I will try to do a PR if you and marcbria like these suggestions

marcbria commented 5 years ago

I don't think it should be the containers reponsability to verify if the database exists, we're adding some logic to a simple bootstraping script. Be we can analyse that, if not to complicated using mysql client over bashscript. If you have time, please fell free to make an PR.

I agree with you Lucas. We don't know how people will use this docker so, the simplest is the better. If somebody is interested in adding more logic, it can be done with additional scripts (as I'm doing with "dojo").

But @Potomac is right in the fact that we need to improve the installation script... A ojs-cli-install is a feature request so it will be developed someday (the curl method is far from been the best solution).

What do you think if we keep simple in the docker side but we ask PKP to improve the install script to check this kind of conditions?

lucasdiedrich commented 5 years ago

I think its better, this script should also be used over others PKP project, is there a change of @Potomac help develop it over PKP branch @marcbria ?

I actually doesn't know how is the developing proccess of PKP.

marcbria commented 5 years ago

... is there a change of @Potomac help develop it over PKP branch @marcbria? Of course, PKP accepts PRs from external contributors. You only need to clone pkp-ojs' repo, and "pull request" to them with the changes you like. PKP dev team will review the proposal and make suggestions in a loop, till is approved and published in the next official release.

Potomac54 commented 5 years ago

Hello,

I made these suggestions because I want a kind of "plug and play" docker image of OJS, I think some manual steps can be avoided (for example the fact that the user must set the OJS_CLI_INSTALL environment variable to 1 when first running the image),

the image should be able to manage and guess automatically 2 scenarios and run the appropriate script(s) :

another option is to recreate the config file at each start of the image (supervisord, ojs-pre-start script), with sed commands, by using environment variables, most users will probably never change the options in config.file.php after an installation of OJS, so the storage of config.inc.php in a persistent volume is not really needed in this use case, options of config.inc.php can be provided via environment variables defined in Dockerfile for example

Potomac commented 5 years ago

I added a pull request which add this feature of automatic installation (no need anymore of OJS_CLI_INSTALL), and the feature of automatic recreation of config file (no need to provide volume for config.inc.php) :

https://github.com/lucasdiedrich/ojs/pull/31/commits/cd4f3aa350538e8198f3662ae47f29326c214409