Added docker environment #617
Conversation
There was a problem hiding this comment.
Left some comments marking areas where the docker setup would break local setups. Note that this isn't just so that it's convenient for people who don't want to use Docker, as is these changes would likely break our deployment :).
Also, it seems like the tests (which to be fair are rather outdated) are broken because they reference some /app path.
Finally, I might make a few small tweaks to the README myself before merging. If you want, I can also handle all the changes to that (including addressing the comment below about keeping the old instructions). Just lmk!
Thanks again for doing this work!
| @@ -4,172 +4,42 @@ This repository holds the code for [wikispeedruns.com](https://wikispeedruns.com | |||
|
|
|||
| ## 1. Prerequisites | |||
|
|
|||
| - Python 3.7 or greater | |||
There was a problem hiding this comment.
Maybe we can move this to a separate ReadMe linked from this main one. I think it's still helpful to have for those who aren't familar with Docker, as well as essentially documentation for what Docker is doing.
There was a problem hiding this comment.
There are also some optional steps to setup features like achievements and such that could be useful still.
|
|
||
| app.json_encoder = CustomJSONEncoder | ||
|
|
||
| app.config.from_file('../config/default.json', json.load) | ||
| app.config.from_file('/app/config/default.json', json.load) |
There was a problem hiding this comment.
Would need to change this here to reference base_dir as well so that the local builds don't break.
| image: mysql:8 | ||
| environment: | ||
| MYSQL_ROOT_PASSWORD: rootpassword | ||
| MYSQL_USER: testuser |
There was a problem hiding this comment.
why not just setup the user here?
| @@ -1,7 +1,7 @@ | |||
| { | |||
| "SECRET_KEY": "ahhhhhhhhhh", | |||
|
|
|||
| "MYSQL_HOST": "127.0.0.1", | |||
| "MYSQL_HOST": "mysql", | |||
There was a problem hiding this comment.
This also assumes a Docker setup, which while is the default, doesn't mean we should break other dev environments. I think the best way to handle this would be to have a config/docker.json with this Docker setup, and then set some environment variable in docker or something to signal the application to use that config.
| @@ -10,12 +9,15 @@ | |||
|
|
|||
| import bcrypt | |||
|
|
|||
|
|
|||
| docker_fp = "/app/config/default.json" | |||
There was a problem hiding this comment.
Same as other comments, this should be redone either using __file__ or a command line argument to get the path.
Co-authored-by: Daniel Qian <[email protected]>
Made some small changes, fixed docker filepaths with
__file__.LMK if there's anything else :)