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

Updated Travis scripts #12

Merged
merged 1 commit into from
Dec 20, 2018
Merged

Updated Travis scripts #12

merged 1 commit into from
Dec 20, 2018

Conversation

clintonb
Copy link
Member

@clintonb clintonb commented Dec 17, 2018

  • We now pull images prior to building. This helps avoid wasting time completely rebuilding the app container.
  • Added test to ensure migrations have been generated, and run properly.
  • We wait for the database to become available before running Django management commands

@TangoYankee
Copy link
Member

What do you think is causing: psycopg2.OperationalError: could not connect to server: Connection refused?

@clintonb
Copy link
Member Author

The database container hasn’t started. I thought I’d resolved this.

@TangoYankee
Copy link
Member

Yeah, my understanding is still too limited to make a meaningful contribution

- We now pull images prior to building. This helps avoid wasting time completely rebuilding the app container.
- Added test to ensure migrations have been generated, and run properly.
- We wait for the database to become available before running Django management commands
@clintonb clintonb force-pushed the clintonb/travis-updates branch from 6d03077 to 78362b1 Compare December 19, 2018 22:35
@clintonb
Copy link
Member Author

This is ready. @TangoYankee or @r-b-g-b, please approve so that I can merge.

@r-b-g-b
Copy link
Collaborator

r-b-g-b commented Dec 20, 2018

Nice! Looks like it's behaving better now. One issue I read about involves wait-for-it.sh and the way the postgres docker containers are instantiated. wait-for-it.sh just checks that a given host:port responds. While initializing itself, the postgres container starts and stops several times before it is actually ready. It is possible that wait-for-it.sh falsely detects the postgres service during one its intermediate phases. No sense for how likely this is to ever occur, just something to keep in mind if we ever notice random failed tests. I think this looks ready to merge, thanks @clintonb !

Makefile Outdated Show resolved Hide resolved
.travis.yml Outdated Show resolved Hide resolved
@clintonb
Copy link
Member Author

@r-b-g-b your concern is valid in general, but not an issue in this specific case. The restart issues only apply on the first run of the database. The first line of script starts both containers. We don't actually need database access until the calls to the migrate and make targets. The Postgres container initialization is relatively quick, and is completed by the time we reach those lines in the script.

I imagine this race condition will be fine for the foreseeable future. If any drastic changes are made to that container, we should definitely revisit this issue.

@clintonb clintonb merged commit bcc1dc4 into master Dec 20, 2018
@clintonb clintonb deleted the clintonb/travis-updates branch December 20, 2018 06:30
@exchrotek exchrotek mentioned this pull request Mar 2, 2022
6 tasks
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.

3 participants