Skip to content

Add docker support#186

Open
alexanderfletcher wants to merge 1 commit intoAdmiralGT:mainfrom
alexanderfletcher:docker
Open

Add docker support#186
alexanderfletcher wants to merge 1 commit intoAdmiralGT:mainfrom
alexanderfletcher:docker

Conversation

@alexanderfletcher
Copy link
Copy Markdown
Contributor

wanted to spin up something so just got this spun up, I haven't really used django or python in prod much before so let me know if any of this is wrong

  • spins up postgres db
  • runs the migrations against it
  • spins up the app and makes available at port 8000
  • also spins up a simple pg client but unsure if this is needed

note: theres weird names for some of the env variables as i use the production config and just give them differnet values. Id like to change this so these env vars are clean but didnt want to change anything for production in this PR : )

ports:
- 5432:5432

adminer:
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Pretty sure this isn't required so I think we can just get rid of it.

@AdmiralGT
Copy link
Copy Markdown
Owner

Do you have a proposal for changing the production environment variables? I think I'd be keen to instead have another settings file, possibly something like docker.py and then use that as part of the docker config which could simply the naming schemes.

@alexanderfletcher
Copy link
Copy Markdown
Contributor Author

Do you have a proposal for changing the production environment variables? I think I'd be keen to instead have another settings file, possibly something like docker.py and then use that as part of the docker config which could simply the naming schemes.

generally with docker files you want to emulate whats in production as closely as possible, I also dislike config files in general as its just another thing thats different to prod,

I think itd be pretty simple to just change the production config slightly and change the env vars, for exmaple atm in prod it does some fancy string manipulation to get the postgres user and stuff like that where that info could just be capture in the env variable

@AdmiralGT
Copy link
Copy Markdown
Owner

AdmiralGT commented Jul 8, 2022

Do you have a proposal for changing the production environment variables? I think I'd be keen to instead have another settings file, possibly something like docker.py and then use that as part of the docker config which could simply the naming schemes.

generally with docker files you want to emulate whats in production as closely as possible, I also dislike config files in general as its just another thing thats different to prod,

The different settings options though exist to support the different environments in which the application is running. You say you spun this up, did you try and upload a script to the database with a PDF? I suspect that will fail because botc.production is expecting particular Azure Blob Storage resources to exist which isn't in this case and it will just fail. You won't be able to download scripts either because of some janky code in the download_pdf function https://github.com/AdmiralGT/botc-scripts/blob/main/scripts/views.py#L518

Also at the moment I do allow production to enter DEBUG mode with an environment variable but I should probably remove that. That would mean the docker environment wouldn't have useful debug error messages in the web browser such as seeing what's wrong with 404 errors, exceptions etc.

So I think my preference would still be to store this as a separate docker.py in settings, and then we can get rid of most of the environment variables other than DJANGO_SETTINGS.

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.

2 participants