Conversation
rjnienaber
left a comment
There was a problem hiding this comment.
Bear in mind that I was testing this on Linux so some of these comments might be due to it being different from Mac.
| APP_DEBUG=true | ||
| APP_LOG_LEVEL=debug | ||
|
|
||
| DB_HOST=mysql # this is the name of the compose database service |
There was a problem hiding this comment.
I think the comment interferes with the artisan migration and I was getting an error until I removed it. It was creating a DSN that looked like the following:
mysql:host=mysql # this is the name of the compose database service;port=3306;dbname=hotsapi
instead of
mysql:host=mysql;port=3306;dbname=hotsapi
| MYSQL_USER: 'docker' | ||
| MYSQL_PASSWORD: 'docker' | ||
| healthcheck: | ||
| test: ['CMD', '/opt/goss', '-g', '/opt/goss/goss-mysql.yaml', 'validate'] |
There was a problem hiding this comment.
Should the command be /opt/goss/goss ? Otherwise it just seems to be pointing at a directory.
| service: | ||
| mysql: | ||
| enabled: true | ||
| running: true |
There was a problem hiding this comment.
not sure if the service field has been auto-generated but mysql runs in the docker container as pid 1 and doesn't use a service manager.
There was a problem hiding this comment.
Yup, I generated this with the Goss autoadd, I'll find an alternative
| gid: 999 | ||
| process: | ||
| mysql: | ||
| running: true |
There was a problem hiding this comment.
I think the process name in this case is mysqld. Also, would it make sense to add a section that waits for a listener on port 3306?
e.g.
tcp:3306:
listening: true
ip:
- 0.0.0.0
| @@ -0,0 +1,5 @@ | |||
| # Goss | |||
| Goss is a standalone Go binary similar to servspec, which is used to validate server state: <https://github.com/aelsabbahy/goss> | |||
|
|
|||
There was a problem hiding this comment.
Perhaps it would be better if the goss binary is downloaded from the release page when script/setup is called (if it's not available)?
There was a problem hiding this comment.
Yea this is a better method that including it in the repo, I'll add download command to the setup script.
| cd "${ROOT}" | ||
| cp .env.example.docker .env | ||
|
|
||
| docker-compose up |
There was a problem hiding this comment.
Is there a missing step here? I had to run the following docker build command to ensure the hotsapidev/hotsapi:dev image was available:
$ docker build -t hotsapidev/hotsapi:dev -f docker/Dockerfile.hotsapi .
There was a problem hiding this comment.
Hm... docker-compose up should build that missing image if it doesn't already exist as part of the up command. Let me delete all my images and see what happens.
1b29461 to
db5a6e4
Compare
|
this one will take some time to review and test |
|
Do we really need this level of complexity/automation? Why not just let developer make a few steps manually according to docs? Like
It's not that hard doing manually but will avoid some complexity in code. Also for dev environment it doesn't make much sense to keep worker, scheduler, parser, prune services. They can be tested by running corresponding artisan commands from console. Memcached could be turned off because we have In my opinion writing a better docs and keeping code simple is a better way. I'll try to write a manual how to set up dev environment. We might need this if we decide to implement some kind of staging environment that runs integration tests or something though. |
|
@poma definitely get your point in regards to added complexity, but it simplifies environment setup following a common pattern here: https://github.com/github/scripts-to-rule-them-all. I'm fine leaving it for later but I'd argue that lowering the barrier to entry would make it easier to get contributors, and we can always document and automate how the system works. To address a couple other points discussed out of band:
|
|
That I've been working a bit on migrations and developer setup over the last couple of days which means I've been setting up and blowing away the dev environment quite a bit. Including a wait in poma's setup steps has been working well for me. |
Apologize for the large PR, I didn't want to touch files that were already there, as I'm not sure what role they play in the CI workflow yet, so some things have been duplicated and edited, instead of changed in place.
This PR does a few things:
docker-compose upI tested these changes locally, and was able to get everything to spin up and access the web frontend, however I tried uploading a replay file and seeing an error https://share.getcloudapp.com/mXum1j60. I don't think that error is related to my changes and this changeset is large enough, so I planned to tackle this in another PR.