Refactor utility script to allow for future services #707
Conversation
DiamondJoseph
requested review from
Relm-Arrowny,
callumforrester and
stan-dot
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #707 +/- ##
=======================================
Coverage 92.17% 92.17%
=======================================
Files 35 35
Lines 1803 1803
=======================================
Hits 1662 1662
Misses 141 141 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
|
please then just add it into see this https://github.com/DiamondLightSource/authz/tree/main/.devcontainer |
|
I agree with @stan-dot, automatic support in the devcontainer would be very nice |
src/script/start_services.sh
Outdated
There was a problem hiding this comment.
I didn't catch this when it was first checked in but I don't think these files belong in src, they are config for the dev environment, not source code.
|
re: PR title phrasing https://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it |
I'm going to need it for system tests with authentication. |
your initial phrasing didn't include this urgency |
DiamondJoseph
force-pushed
the
docker-compose
branch
2 times, most recently
from
5c6dd18 to
90c1ca1
Compare
|
looking into it |
|
|
having set up in |
|
@garryod please help |
|
@stan-dot seems to work on my machine, have you pulled the latest version? |
|
yeah I did @DiamondJoseph maybe it's about the setup and env variables? |
|
I'm beginning to think that the combination of docker-compose/podman/devcontainers is quite broken at the moment for multi-stage builds, see microsoft/vscode-remote-release#10178 This doesn't affect the example @stan-dot linked from the analysis team because that isn't a multi-stage build. Tagging @gilesknap in case he has any suggestions. |
|
I found the combination broken when I last tried it and decided it was too fragile to use. A conversation with @garryod revealed that older versions of docker-compose did work. But that makes it sound like it's going in the wrong direction at present. |
DiamondJoseph
force-pushed
the
docker-compose
branch
from
c59b15a to
f713a20
Compare
|
@DiamondJoseph we can still use docker-compose to manually initialize rabbit + the other dependant services, as was your original plan. |
|
@callumforrester after I'm done working on what I currently am I'm going to revisit this to see if we can split the Dockerfile into the developer Dockerfile, and use what is currently produced by the python-copier-template, and the Dockerfile for running the service, see if it behaves with podman. |
|
Okay, although I still share @gilesknap's concern about the general direction of upstream |
stan-dot
force-pushed
the
docker-compose
branch
from
f713a20 to
6c34338
Compare
DiamondJoseph
force-pushed
the
docker-compose
branch
from
6c34338 to
2a0f031
Compare
DiamondJoseph
force-pushed
the
docker-compose
branch
from
2a0f031 to
5a16d1a
Compare
DiamondJoseph
force-pushed
the
docker-compose
branch
from
5a16d1a to
b778bb9
Compare
There was a problem hiding this comment.
Should: To avoid me having to do another huge rebase, can we get #674 in first and then rebase to suit this (pleeease)?
There was a problem hiding this comment.
If you can address my comments ;P
| # with docker | ||
| docker compose -f .devcontainer/compose.yml up | ||
| # or with podman-compose | ||
| podman-compose -f .devcontainer/compose.yml up |
There was a problem hiding this comment.
Should: Although we gave up on autostarting these services in the devcontainer, can we make docker-compose available in the devcontainer so the developer can do it manually? Otherwise this whole section doesn't apply if you're using a devcontainer.
Tested and it works on your Diamond and personal machines @stan-dot? |
|
no I didn't test it such as my devcontainer setup fails with any repo today for some reason |
Move RabbitMQ docker command into a docker-compose file, to allow defining multiple dependent services at once.
This may be used for mock authentication later.