Closed dhruvkb closed 1 month ago
load_to_s3
I could not find any occurrences of this string in the project except for thedocker-compose.yml
file. This leads me to suspect it might actually be completely unused.
This service loads some testing data into the local minio instance, which is used currently for iNaturalist but could be expanded in the future. It should not be removed (unless we can find a similar mechanism for loading data into S3 on startup that doesn't involve a separate image).
@AetherUnbound thanks for clarifying! There might be ways to do this that don't involve a dedicated service but at the very least documenting this on the service itself would be helpful. It's also my bad that I did not look closely enough before making the assumption.
Also I don't know if this is an antipattern but if so, could you make an issue for us to investigate and find a more elegant solution?
The purpose of the API nginx image is to test the Nginx configuration we run in the live environments. We can make it optional by removing it from the api compose profile, (maybe put it in something like api-extra
?), but it absolutely should not be removed.
We've discussed several times in passing getting rid of the proxy
one for the API. I would support doing so: we don't need to regularly test the local API with HTTPS. We need to keep the other API nginx one for being able to easily verify changes to the API's nginx configuration, which do indeed happen on occasion, especially logging changes. Example: #4569
@sarayourfriend the proxy
service was removed about 2 months ago in #4605.
Good to know that the nginx
image should not be removed. We can close this issue as we've already achieved the goal of pruning unused services or knowing where these services are used.
Problem
The following services in Docker Compose are largely unused.
load_to_s3
I could not find any occurrences of this string in the project except for thedocker-compose.yml
file. This leads me to suspect it might actually be completely unused.nginx
This runs thenginx
target from the APIDockerfile
. It's purpose in the local dev environment is unclear and building it when initialising the API slows down CI.proxy
proxy
was added long back to test for HTTPS with the API server during local development. It also introduces a dependency onmkcert
so removing it will have twice the benefit.They should be removed or, if they are in fact used, comments should be added to list down what they are used for and where.
Additional context
This is part of a bigger improvement of the Docker Compose configuration that involves upgrading to v3 #4046 and is closely linked to #1910. This was also raised by @krysal in #1009.