Https server #2
Https server #2
Conversation
Bumps [django](https://github.com/django/django) from 2.2.18 to 2.2.24. - [Release notes](https://github.com/django/django/releases) - [Commits](django/django@2.2.18...2.2.24) --- updated-dependencies: - dependency-name: django dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]>
…django-2.2.24 Bump django from 2.2.18 to 2.2.24
Bumps [django](https://github.com/django/django) from 2.2.24 to 2.2.28. - [Release notes](https://github.com/django/django/releases) - [Commits](django/django@2.2.24...2.2.28) --- updated-dependencies: - dependency-name: django dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]>
Bump django from 2.2.24 to 2.2.28
> python manage.py runsslserver 127.0.0.1:9000
Added docker instructions + config
There was a problem hiding this comment.
A couple of other comments:
- In
requirements-dev.inyou can remove docker-compose - I have stopped including
pip-toolsin my dev requirements.txt as it just always seems to lead to odd problems. I don't think it belongs there, same as you wouldn't put pip itself in your requirements.
Dockerfile
Outdated
| EXPOSE 9001 | ||
| CMD python manage.py runsslserver 0.0.0.0:9001 | ||
|
|
||
| RUN apk update && apk upgrade |
There was a problem hiding this comment.
apk update/upgrade is already executed near the top of the Dockerfile. This should be removed.
Dockerfile
Outdated
| EXPOSE 9001 | ||
| CMD python manage.py runsslserver 0.0.0.0:9001 |
There was a problem hiding this comment.
You can expose more than one port but you can't have multiple CMD directives. This would need to be done with a runtime env var or something. Do we actually still need to support both? If not I would simply remove the non-ssl port 9000 version. The image this project produces is only ever used for local dev. We'd just need to make sure the docker compose configs for porta/porta-auto get updated to use https in the CAS_SERVER_URL setting.
| hooks: | ||
| - id: black | ||
| args: | ||
| # don't actually format, just exit with 0 (no changes) or 1 (changes needed) | ||
| - --check | ||
| - casdemoserver | ||
| - hkey | ||
| - repo: https://gitlab.com/pycqa/flake8 |
There was a problem hiding this comment.
This needs to get changed to pull from github. the gitlab url makes me authenticate and is a known "bug". https://www.reddit.com/r/Python/comments/yvfww8/flake8_took_down_the_gitlab_repository_in_favor/
| repos: | ||
| - repo: https://github.com/psf/black | ||
| rev: 20.8b1 # not a "beta"; just weird versioniong | ||
| rev: 22.3.0 |
There was a problem hiding this comment.
i have been switching to the local repo config method for pre-commit. Instead of having to specify a version and repo url for black and flake it just uses whatever you currently have installed in the virtualenv. It's soooo much easier and I cannot understand why it isn't the default. AFAICT there's no downside.
See my recent PR for the porta package updates for an example.
| @@ -12,31 +12,54 @@ pre-commit install | |||
| ## Run locally using docker | |||
|
|
|||
| ```bash | |||
| docker build -t demo-cas-server . | |||
| docker-compose up | |||
There was a problem hiding this comment.
there's no docker compose config here so not sure where this came from
| @@ -1,6 +1,7 @@ | |||
| #!/bin/sh | |||
|
|
|||
| SCRIPTDIR=$(dirname $(readlink -f $0)) | |||
| #SCRIPTDIR=$(dirname $(readlink -f $0)) | |||
| SCRIPTDIR=./demo_data | |||
There was a problem hiding this comment.
the point of the command to figure out the $SCRIPTDIR was so that it didn't matter what your cwd was when running the script. Was it not working?
| # | ||
| # This file is autogenerated by pip-compile | ||
| # To update, run: | ||
| # This file is autogenerated by pip-compile with Python 3.9 |
There was a problem hiding this comment.
why isn't requirements-dev.txt also being updated?
| @@ -12,31 +12,54 @@ pre-commit install | |||
| ## Run locally using docker | |||
|
|
|||
| ```bash | |||
| docker build -t demo-cas-server . | |||
There was a problem hiding this comment.
The example image tag really should be hdce/hkey-demo-cas-server so that the image gets the name used in the porta/porta-auto/doki docker compose configs
No description provided.