Skip to content
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

Switch murmur container to mumble-voip/mumble-docker #1662

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

dylanmtaylor
Copy link
Contributor

Pull request

Purpose
This is a work in progress that I'm submitting for comments. It should address #1635.

Approach
How does this change address the problem?

This replaces the image while trying to keep the old variables the same so things don't break on upgrade for users.

@github-actions github-actions bot added the repo Automatic label label Aug 9, 2023
@nemchik
Copy link
Member

nemchik commented Oct 9, 2023

Sorry for taking so long to get to this. PR looks good to me, but based on the volume changes I have a question. Will the existing user data be used by the new container? or will users have to shuffle around some of the files in their config folder to have the new container pick them up?

Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@dylanmtaylor
Copy link
Contributor Author

Sorry for taking so long to get to this. PR looks good to me, but based on the volume changes I have a question. Will the existing user data be used by the new container? or will users have to shuffle around some of the files in their config folder to have the new container pick them up?

I'm doubtful that data will transfer over cleanly.

@github-actions github-actions bot added the apps Automatic label label Feb 20, 2024
@nemchik
Copy link
Member

nemchik commented Feb 20, 2024

I do like the idea of switching to this image, but I also wonder if we should just make a new app folder with the name mumbleserver and deprecate this one as the image hasn't been updated in years. There are a few additional environment vars like the UID/GID that we could also include with the new image. I'm also working on a pretty insanely massive PR with #1729 that has breaking changes (which I try to mitigate as much as possible) for end users, including a handful of deprecations (and new apps). So maybe it could be tucked into that, or wait until that merges since the naming cadence for variables (and thus labels) is changing.

@dylanmtaylor
Copy link
Contributor Author

I do like the idea of switching to this image, but I also wonder if we should just make a new app folder with the name mumbleserver and deprecate this one as the image hasn't been updated in years. There are a few additional environment vars like the UID/GID that we could also include with the new image. I'm also working on a pretty insanely massive PR with #1729 that has breaking changes (which I try to mitigate as much as possible) for end users, including a handful of deprecations (and new apps). So maybe it could be tucked into that, or wait until that merges since the naming cadence for variables (and thus labels) is changing.

If you want to work this image change into your breaking PR I'm fine with that. I'm honestly not sure how we can do this in a way which is least annoying to users of mumble. I was trying to seamlessly swap over the container so it gets recreated without them having to redo their configuration but the data structure is different.

Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apps Automatic label no-pr-activity repo Automatic label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants