-
Notifications
You must be signed in to change notification settings - Fork 3k
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
fix(postgres): postgres setup #3092
fix(postgres): postgres setup #3092
Conversation
docker/postgres-setup/Dockerfile
Outdated
@@ -6,6 +6,6 @@ COPY docker/postgres-setup/init.sql /init.sql | |||
COPY docker/postgres-setup/init.sh /init.sh | |||
RUN chmod 755 init.sh | |||
|
|||
ENV DATAHUB_DB_NAME="datahub" | |||
ENV DATAHUB_DB_NAME=$POSTGRES_DATABASENAME |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need a separate env variable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we just use DATAHUB_DB_NAME ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we don't need it. Initial intention is for the users to be able set their own database name through the env. But if it's unnecessary, we can remove it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So DATAHUB_DB_NAME already does that. Here it seems like it's always set to "datahub" but it does use the appropriate db name during setup.(This is the case for mysql-setup, where I tested with different db names)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
true...dbeec80
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR fixes the error in the init.sql. Postgres SQL doesn't support
CREATE DATABASE IF NOT EXISTS
, so I have added a psql shell script in the init.sh to check whether there's the database we want, if notcreate database datahub
. And also some minor changes in the sql file. Such as we can use the TEMP table syntax when we create a temp table.Checklist