-
Notifications
You must be signed in to change notification settings - Fork 794
Remove backup classes #1407
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
Remove backup classes #1407
Conversation
We are currently planning work on doing a complete revamp of backups as soon as the team get a chance. See epic here: https://tickets.puppetlabs.com/browse/IAC-1661 |
|
@LukasAud Could you please take a look at this one. It is similar in nature to the one you just merged. |
Is it? mysql::server::backup is documented in the README, and is not related to unsupported Operating Systems. There's also #1447 open where the author has been asked to rebase so that the change can be reviewed properly before merging. I'm against removing this functionality which I assume actually has a fair few users and seen willingness from community members to contribute fixes and enhancements. |
It is removing more cruft :) @alexjfisher Check out the ticket for more info. Basically backups are too specialized to the user's environment so an example would be better than actual code. Also we already have db backup at VP. |
One person's cruft is another's functionality they rely on. Unlike the solaris code, this is out of the way and not interfering with non users. |
Hi @ghoneycutt, sorry for the long delay in feedback. The ping must have gotten lost in my emails. This is considered a major change and at the moment we do not have the resources to dedicate time to this. It seems to break (understandably) a large amount of test cases in all of our supported OS, which in turn adds to the effort required. In addition, we are currently not sure if this is the approach we want to follow regarding the backup classes. @alexjfisher makes a good point stating that this might be useful functionality for a portion of the community and/or clients but not all. The suggested change will require an extensive discussion and/or extra work before we can move ahead and merge/close it. |
I strongly disagree. They are very useful, at least for our use-cases.
FWIW, pupppet-dbbackup is not a valid replacement in our case. It requires systemd and does not support xtrabackup. |
Hi @ghoneycutt. Unfortunately, it doesn't look like we will be able to investigate your PR in the near future, as we are currently focusing our work on other areas. We are also afraid this PR will keep being flagged as inactive by our system. As such, could you create a feature request in our 'Issues' section instead and then close and link the PR there? That way, this request will remain in our view and any progress/discussion will still be easily accessible. Sorry for the inconvenience. |
Closing in line with above comments. |
Suggest that we remove these backup related classes. The work is useful and should be kept as example profiles. Backups are extremely site specific and having them in their current state is not providing value. These would do better converted to example profiles to show how you can do backups using these different methods. Moving them to examples also makes the module easier to maintain.