Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make auto db creation optional with DB_CREATE #558

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Make auto db creation optional with DB_CREATE #558

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -622,6 +622,7 @@ Below is the complete list of parameters that can be set using environment varia
- **REDMINE_AVATAR_SERVER_URL**: Avatar server for displaying user icons. Defaults to `https://www.gravatar.com`
- **DATABASE_URL**: The database URL. See [Configuring a Database](https://guides.rubyonrails.org/configuring.html#configuring-a-database). Possible schemes: `postgres`, `postgresql`, `mysql2`, and `sqlite3`. Defaults to no URL.
- **DB_ADAPTER**: The database type. Possible values: `mysql2`, `postgresql`, and 'sqlite3'. Defaults to `mysql`.
- **DB_CREATE**: Whether the db should be automatically created (`bundle exec rake db:create`). Defaults to `true`.
- **DB_ENCODING**: The database encoding. For `DB_ADAPTER` values `postresql` and `mysql2`, this parameter defaults to `unicode` and `utf8` respectively. For full unicode support (all emojis) with mariadb or mysql set this to `utf8mb4` and make sure to also set all tables to `utf8mb4` and use `collate utf8mb4_unicode_ci`. Existing databases can be converted by following this [HowTo](https://www.redmine.org/projects/redmine/wiki/HowTo_convert_a_database_from_utf8_to_utf8mb4).
- **DB_HOST**: The database server hostname. Defaults to `localhost`.
- **DB_PORT**: The database server port. Defaults to `3306`.
Expand Down
21 changes: 20 additions & 1 deletion assets/runtime/functions
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1048,8 +1048,27 @@ migrate_database() {
CACHE_VERSION=
[[ -f ${REDMINE_DATA_DIR}/tmp/VERSION ]] && CACHE_VERSION=$(cat ${REDMINE_DATA_DIR}/tmp/VERSION)
if [[ ${REDMINE_VERSION} != ${CACHE_VERSION} ]]; then
if [[ "${DB_CREATE:-true}" == "true" ]] ; then
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

defaults should all be set in assets/runtime/env-defaults for consistency

echo "Creating database..."
# Disable "exit on error" so we can capture the error code
set +e
exec_as_redmine bundle exec rake db:create >/dev/null
res=$?
# Restore "exit on error"
set -e

if [[ 0 -ne $res ]] ; then
Comment on lines +1053 to +1060
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note shellcheck will tell you its better to if check the command directly instead of storing the results. I think this also would mean we don't have to disable exit on error.

Suggested change
# Disable "exit on error" so we can capture the error code
set +e
exec_as_redmine bundle exec rake db:create >/dev/null
res=$?
# Restore "exit on error"
set -e
if [[ 0 -ne $res ]] ; then
if ! exec_as_redmine bundle exec rake db:create >/dev/null
then

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note shellcheck will tell you its better to if check the command directly instead of storing the results. I think this also would mean we don't have to disable exit on error.

I wanted to retain the exit value from rake db:create, since I don't know the range of non-zero values that it can produce. The only way I know of doing that is to do the res=$?, which requires disabling "exit-on-error".

I'm happy to change it you are ok with returning a fixed value, e.g.:

if ! exec_as_redmine bundle exec rake db:create >/dev/null ; then
  echo <<-EOM
Failed to create db. If db exists, and user (or role) exists and has access to db but no permission to create databases, set:
-----------------------
DB_CREATE=false
-----------------------
EOM

       # FWIW, using "return" here would be consistent a similar use of "exec_as_redmine' in "redmine_check_database_connection"
        exit 1
fi

echo <<-EOM
Failed to create db. If db exists, and user (or role) exists and has access to db but no permission to create databases, set:
-----------------------
DB_CREATE=false
-----------------------
EOM
return $res
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be an exit if we are to keep the previous behavior. As is, this will skip db migration and then redmine will continue to try to start, likely leading to more confusing errors.

fi
fi

echo "Migrating database. Please be patient, this could take a while..."
exec_as_redmine bundle exec rake db:create >/dev/null
exec_as_redmine bundle exec rake db:migrate >/dev/null

# clear sessions and application cache
Expand Down