generated from opensafely-core/repo-template
-
Notifications
You must be signed in to change notification settings - Fork 0
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
Dockerise the CLI #11
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The setuptools backend works with a single directly without configuration. However when we add more than one directory, which we're about to do, it needs to be configured to know where to find the code we want to package.
We're not using this entrypoint, and it was incorrectly added with the add_job name, after copy/pasting from job-runner.
ghickman
force-pushed
the
docker
branch
8 times, most recently
from
November 7, 2023 12:21
3df1856
to
ef8f6b5
Compare
We're using specific names for all the other configuration env vars, and have multiple databases involved with this project, so lets make it clear which database this URL is for.
So we can rely on it in local configuration.
We have multiple things which need running in this project, so lets name them clearly.
lucyb
reviewed
Nov 7, 2023
ghickman
force-pushed
the
docker
branch
2 times, most recently
from
November 8, 2023 09:23
3d86b4c
to
d8da023
Compare
lucyb
approved these changes
Nov 8, 2023
We switched to using pyproject.toml for this project in fc2050f but this file was missed as part of that. Nothing is configured to use it so it's just a plain deletion.
The former invokes the docker plugin instead of the standalone tool, allowing us to target compose v2 more consistently.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This wraps the CLI tool in docker ready for deployment to a dokku.
I took the job-runner configuration (which is already set up for a CLI tool) and adapted it for this repo. I've removed all the bits that I thought were job-runner specific (in particular things like where does the docker socket live on the host, SSH config, etc).
Since this is the main tool of the repo I've tried out leaving the docker config in the
docker
directory but moving the invocations up to the main justfile, and putting the docker-compose bits into the existingdocker-compose.yaml
. Can anyone think of any problems or confusion this might cause?I've done a little tidying along the way, most notably set up a static username and db name for timescaledb, and put that configuration into the
dotenv-sample
which both docker and the justfile are now autoloading.You should be able to test this with
just docker-run dev python -m metrics
and get the help text thatjust metrics
gives you.I suspect we'll have some bugs to iron out once we start running this in production but I don't know how to start testing it without merging this first.