-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat(dev): express dependencies better #11175
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
Conversation
Currently, a developer is expected to perform `make serve` prior to running `make initdb`, as the `serve` command will start **all** services expressed in `docker-compose.yml`. By using a `healthcheck` on `db` that uses a built-in CLI tool to test for health, and Docker Compose 3.9 Long Syntax for the conditions, we can now run `make initdb` in one command, followed by `make serve`. This enables pre-build behaviors to establish the `postgres` and `elasticsearch` databases prior to other apps starting. Refs: https://docs.docker.com/compose/compose-file/#healthcheck Refs: https://www.postgresql.org/docs/10/app-pg-isready.html Refs: https://docs.docker.com/compose/compose-file/#long-syntax-1 Signed-off-by: Mike Fiedler <[email protected]>
Removes the secondary terminal session as a result! Signed-off-by: Mike Fiedler <[email protected]>
@di This is a pre-step to adding a I'd encourage any reviewer to |
@miketheman Somewhat related, have you seen #9993? Any thoughts on how that would relate here? |
@di I recall reading that a little while ago, and I think that depending on what it is we're trying to optimize for will change the direction of either this PR or that one. If the purpose is to minimize the interaction a developer had to take on their initial usage, then that PR is a good idea to pursue, potentially independent of this one. If we leverage Gitpod as a cloud development platform, then it kind of sidesteps the issue, as the database could be initialized during the prebuild phase, and be ready for the developer. It's entirely possible that we ought to merge this and then take another pass at the other PR. |
I agree! |
Good catch. Although I wouldn't depend on Elastic if it takes too much memory, depending on DB will certainly solve the problem of running initial migrations. They probably failed if the DB didn't start before web. I will try to revisit #9993, which should speed up DB init, or at least it won't be up until the DB created and SQL dump is loaded, so one less point of failure. |
* feat(dev): express dependencies better Currently, a developer is expected to perform `make serve` prior to running `make initdb`, as the `serve` command will start **all** services expressed in `docker-compose.yml`. By using a `healthcheck` on `db` that uses a built-in CLI tool to test for health, and Docker Compose 3.9 Long Syntax for the conditions, we can now run `make initdb` in one command, followed by `make serve`. This enables pre-build behaviors to establish the `postgres` and `elasticsearch` databases prior to other apps starting. Refs: https://docs.docker.com/compose/compose-file/#healthcheck Refs: https://www.postgresql.org/docs/10/app-pg-isready.html Refs: https://docs.docker.com/compose/compose-file/#long-syntax-1 Signed-off-by: Mike Fiedler <[email protected]> * docs: reorder commands to reflect usage Removes the secondary terminal session as a result! Signed-off-by: Mike Fiedler <[email protected]>
Currently, a developer is expected to perform
make serve
prior torunning
make initdb
, as theserve
command will start allservices expressed in
docker-compose.yml
.By using a
healthcheck
ondb
that uses a built-in CLI tool to testfor health, and Docker Compose 3.9 Long Syntax for the conditions, we
can now run
make initdb
in one command, followed bymake serve
.This enables pre-build behaviors to establish the
postgres
andelasticsearch
databases prior to other apps starting.Refs: https://docs.docker.com/compose/compose-file/#healthcheck
Refs: https://www.postgresql.org/docs/10/app-pg-isready.html
Refs: https://docs.docker.com/compose/compose-file/#long-syntax-1
Signed-off-by: Mike Fiedler [email protected]