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.

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

'Initialization scripts' like in https://hub.docker.com/_/postgres implemented #538

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

yanchenko-igor
Copy link
Contributor

@yanchenko-igor yanchenko-igor commented Jan 8, 2021

This PR allows to add additional script after database initialization.

@@ -1,8 +1,10 @@
#!/bin/bash

cd "$(dirname "${BASH_SOURCE[0]}")" || exit 1
export POSTGRES_USER="$1"
Copy link
Contributor

Choose a reason for hiding this comment

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

The first parameter is actually not a postgres user, but HUMAN_ROLE.

@@ -1,8 +1,10 @@
#!/bin/bash

cd "$(dirname "${BASH_SOURCE[0]}")" || exit 1
export POSTGRES_USER="$1"
export POSTGRES_DB="$2"
Copy link
Contributor

Choose a reason for hiding this comment

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

The second parameter is not the database name, but the connection string in the format "host=localhost port=5432 user=postgres dbname=postgres".

There is one exception though, the post_init.sh could be called from on_role_change.sh. In this case the parameter would be just "postgres".

Comment on lines +212 to +214
ls /docker-entrypoint-initdb.d/ > /dev/null

docker_process_init_files /docker-entrypoint-initdb.d/*
Copy link
Contributor

Choose a reason for hiding this comment

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

Since in the readme you tell that scripts will be executed only for the new cluster, it makes sense to execute this block only if we know that post_init is not executed from on_role_change:

if [ "$2" != "postgres" ]; then
    ...
fi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants