puppetlabs-mysql
puppetlabs-mysql copied to clipboard
puppetlabs
Remove backup classes
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.
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
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.
@LukasAud Could you please take a look at this one. It is similar in nature to the one you just merged.
@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 https://github.com/puppetlabs/puppetlabs-mysql/pull/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.
Backups are extremely site specific and having them in their current state is not providing value.
I strongly disagree. They are very useful, at least for our use-cases.
Also we already have db backup at VP.
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.