Clean up HELM docker #8301
Clean up HELM docker #8301
Conversation
kiendang
force-pushed
the
helm-cleanup-docker
branch
from
248fd11 to
aaab552
Compare
packages/grid/default.env
Outdated
| @@ -27,7 +27,8 @@ TRAEFIK_VERSION=v2.10 | |||
| REDIS_VERSION=6.2 | |||
| RABBITMQ_VERSION=3 | |||
| SEAWEEDFS_VERSION="3.59" | |||
| DOCKER_IMAGE_SEAWEEDFS=chrislusf/seaweedfs | |||
| DOCKER_IMAGE_SEAWEEDFS=openmined/grid-seaweedfs | |||
| DOCKER_IMAGE_SEAWEEDFS_ORIGINAL=chrislusf/seaweedfs | |||
There was a problem hiding this comment.
this is always the same I suppose? If thats true, why do we have a variable for it
There was a problem hiding this comment.
We build our own image for seaweed based on the chrislusf/seaweedfs image (see seaweed.dockerfile) and push it to the registry as openmined/grid-seaweedfs.
There was a problem hiding this comment.
yes, I understand that, but why is it a variable, instead of hardcoded
There was a problem hiding this comment.
ya good point. not very useful here.
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
| @@ -236,17 +236,15 @@ services: | |||
| - blob-storage | |||
| depends_on: | |||
| - proxy | |||
| env_file: | |||
| - .env | |||
| image: "${DOCKER_IMAGE_SEAWEEDFS?Variable not set}:${VERSION-latest}" | |||
| environment: | |||
| - S3_VOLUME_SIZE_MB=${S3_VOLUME_SIZE_MB:-1024} | |||
| - S3_ROOT_USER=${S3_ROOT_USER:-admin} | |||
| - S3_ROOT_PWD=${S3_ROOT_PWD:-admin} | |||
| - S3_PORT=${S3_PORT:-8888} | |||
| - SEAWEED_MOUNT_PORT=${SEAWEED_MOUNT_PORT:-4001} | |||
There was a problem hiding this comment.
MAYBE: Since the seaweedfs dockerfile take the version as an argument,
ARG SEAWEEDFS_VERSION
FROM chrislusf/seaweedfs:${SEAWEEDFS_VERSION}do we need to add it also the docker compose
environment variables including SEAWEEDFS_VERSION
There was a problem hiding this comment.
We might also need to add it also to devspace.yaml, for
seaweedfs deployment
There was a problem hiding this comment.
yes we will have to update that as well.
There was a problem hiding this comment.
we pass the SEAWEEDFS_VERSION as an arg to docker compose build
There was a problem hiding this comment.
Yes we might to take care of devspace, that's true,
Description
Please include a summary of the change, the motivation, and any additional context that will help others understand your PR. If it closes one or more open issues, please tag them as described here.
Affected Dependencies
List any dependencies that are required for this change.
How has this been tested?
Checklist