-
Notifications
You must be signed in to change notification settings - Fork 794
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
Make backup success file path configurable #1207
Conversation
Thanks for submitting the PR @HT43-bqxFqB . |
@HT43-bqxFqB Could you please add tests to verify the new parameter backup_success_file_path. |
Hey @sheenaajay, I have tried to add some minimal default tests, but unfortunately they did not work on my local machine. There for I could only test here with Travis. Locally I got the following error while testing with the PDK:
|
@HT43-bqxFqB Could you please rebase with the latest master. Pdk update has been done recently on this module. Will execute the acceptance tests and let you know if I see the same error.Thank you. |
Just want to say it's not only because /tmp can be erase, but if you try to use nagios/nrpe on a systemd server, the service start with PrivateTmp=true, so the process nrpe cannot access to /tmp or /var/tmp. So we really need this config to be a parameter. |
@HT43-bqxFqB Ran the tests against all OS. Looks Good. Once all comments are incrporated we can merge the changes. |
Thanks alot @HT43-bqxFqB @jas01 |
@HT43-bqxFqB Could you please rebase with the latest master to run the Travis job successfully. Thank you. |
Rebase completed. |
@HT43-bqxFqB Could you please fix the unit tests. Acceptance tests are running clean. Thank you. |
Fix xtrabackup regression introduced in #1207
Using
/tmp
is fine until a node is rebooted and/tmp
is cleared. By offering a configurable parameter, one can change the path to a non temporary one for checks, monitoring etc.Additionally I've added a unit test for the default case. Unfortunately the test did not work on my local machine.