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

Fix setup_db.sh by waiting for pg_isready success return. Fixes #3876 #3908

Merged
merged 8 commits into from
Oct 21, 2021

Conversation

rvacaru
Copy link
Contributor

@rvacaru rvacaru commented Sep 17, 2021

resolves #3876

Description

The setup_db.sh script now waits for the postgres instance to be ready before terminating execution. Code for the creation of dbt database, root roles and grants on dbt has been removed since this is already done in the docker compose. The creation of role noaccess and db dbtMixedCase is kept.

Mind that I didn't successfully run make integration because I get an import error on all the tests on dbt.main. What I tested is that after the script ends I'm able to connect to postgres with:

  • psql postgresql://root:password@localhost:5432/dbtMixedCase
  • psql postgresql://root:password@localhost:5432/dbt

Checklist

  • I have signed the CLA
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have updated the CHANGELOG.md and added information about my change to the "dbt next" section.

@cla-bot cla-bot bot added the cla:yes label Sep 17, 2021
@kwigley kwigley added ok_to_test test postgres Run integration tests for the Postgres adapter in CI. labels Sep 17, 2021
@kwigley
Copy link
Contributor

kwigley commented Sep 17, 2021

forgive me while I close and reopen the PR to kick off the integration test workflow 🤦 , we are still working on a mechanism to re-trigger integration tests!

@kwigley kwigley closed this Sep 17, 2021
@kwigley kwigley reopened this Sep 17, 2021
@rvacaru rvacaru force-pushed the 3876/wait-pg-readiness-make-setup-db branch 3 times, most recently from d5d6a89 to 0f694aa Compare September 24, 2021 09:31
@rvacaru
Copy link
Contributor Author

rvacaru commented Sep 24, 2021

I rebased the branch with develop, that's why the forced push.

@rvacaru rvacaru force-pushed the 3876/wait-pg-readiness-make-setup-db branch from 0f694aa to d0db8e9 Compare October 8, 2021 12:55
test/setup_db.sh Outdated Show resolved Hide resolved
Copy link
Contributor

@jtcohen6 jtcohen6 left a comment

Choose a reason for hiding this comment

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

@rvacaru Thanks for the contribution! The change itself looks good to me. I'm going to move your changelog note up to the topmost section, then we can merge this in :)

@jtcohen6 jtcohen6 merged commit 3ae9475 into dbt-labs:main Oct 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla:yes ok_to_test test postgres Run integration tests for the Postgres adapter in CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wait for postgres docker container to be ready before running commands
4 participants