Skip to content
This repository has been archived by the owner on Aug 1, 2024. It is now read-only.

fix: add names to redis and mongo volumes #1153

Conversation

mohd-akram
Copy link
Contributor

The mongo and redis images provide additional volumes that aren't specified in the compose files. This ensures they have proper names and aren't anonymous.

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Aug 14, 2023
@openedx-webhooks
Copy link

Thanks for the pull request, @mohd-akram! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

@itsjeyd
Copy link

itsjeyd commented Aug 15, 2023

Thanks for this PR @mohd-akram! Aside from needing a rebase, are the changes ready for review?

@itsjeyd itsjeyd added the waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. label Aug 15, 2023
@mohd-akram
Copy link
Contributor Author

@itsjeyd Yup, they are.

@itsjeyd itsjeyd added waiting for eng review PR is ready for review. Review and merge it, or suggest changes. needs test run Author's first PR to this repository, awaiting test authorization from Axim and removed waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. waiting for eng review PR is ready for review. Review and merge it, or suggest changes. labels Aug 16, 2023
@itsjeyd
Copy link

itsjeyd commented Aug 16, 2023

Thanks @mohd-akram, I'm lining this up for a test run now. Once the build is green the changes can go to engineering review.

CC @jmbowman for arch-bom

@itsjeyd itsjeyd added waiting for eng review PR is ready for review. Review and merge it, or suggest changes. and removed needs test run Author's first PR to this repository, awaiting test authorization from Axim labels Aug 22, 2023
@itsjeyd
Copy link

itsjeyd commented Aug 22, 2023

@mohd-akram Looks like we got a green build here, and can proceed with next steps.

@jmbowman This PR is ready for engineering review by arch-bom.

Copy link
Contributor

@rgraber rgraber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rgraber
Copy link
Contributor

rgraber commented Aug 24, 2023

I'm unable to rebase this, but it's good as far as I'm concerned.

@timmc-edx
Copy link
Contributor

timmc-edx commented Aug 30, 2023

Out of curiosity, is there some symptom that this fixes, or is this just to make sure that our docker-compose lists all of the relevant volumes? (Happy to merge either way, once the branch is brought up to date.)

@mohd-akram mohd-akram force-pushed the mohd-akram/add-redis-mongo-volumes branch from 3d46117 to 5d139da Compare August 30, 2023 18:23
@mohd-akram
Copy link
Contributor Author

Yes. Every time you do docker-compose down followed by docker-compose up, it will create a new anonymous volume. Doing this a few times causes the volume list to be littered with nameless volumes providing no clue on what they contain. Providing a name makes docker-compose re-use the same volume while making it clear what the volume contains. I've rebased the branch.

@timmc-edx
Copy link
Contributor

timmc-edx commented Aug 30, 2023

Ah! I'd wondered what all those anonymous volumes were from when I ran docker volume ls. :-)

@timmc-edx timmc-edx merged commit b4de061 into openedx-unsupported:master Aug 31, 2023
10 checks passed
@openedx-webhooks
Copy link

@mohd-akram 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

@timmc-edx
Copy link
Contributor

I was able to confirm the fix -- this is a great little improvement. :-)

@mohd-akram mohd-akram deleted the mohd-akram/add-redis-mongo-volumes branch August 31, 2023 00:44
@mohd-akram
Copy link
Contributor Author

@timmc-edx Thanks!

@itsjeyd itsjeyd removed the waiting for eng review PR is ready for review. Review and merge it, or suggest changes. label Sep 13, 2023
nsprenkle pushed a commit that referenced this pull request Nov 21, 2023
The [mongo](https://hub.docker.com/_/mongo) and [redis](https://hub.docker.com/_/redis) images provide additional volumes that aren't specified in the compose files. This ensures they have proper names and aren't anonymous. This fix should prevent the accumulation over time of anonymous volumes (long hexadecimal names) that show up in `docker volume ls`.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
open-source-contribution PR author is not from Axim or 2U
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants