Skip to content

Conversation

@lukaspieper
Copy link

Closes #6347, #5973

I added the CLI option as discussed in #6347. As I mentioned, I'm not familiar with Rust. I tested my changes to confirm they work, but I don't know anything about Rust best practices.

While implementing this, I noticed that there seem to be three ways to trigger the backup process: via the CLI, the USR1 signal or an API call. Bearing this in mind, I'm wondering whether the current approach is sensible, given that only the CLI will benefit from the change. Might it be more future-proof to add a config entry for the backup path instead of the CLI option to handle all three variants in the same way? For this reason, I am opening this as a draft to discuss this question.

Thanks in advance for spending time on my PR/feature request.

@BlackDex
Copy link
Collaborator

I'm not against this feature, but there are multiple PR's which might have an impact on this.

Also, while the flexibility of a custom directory is nice, i think a per-determined fixed folder would be better. This for example could also help with the S3 feature (not enabled by default) to be useful for these kind of backups.

So, there are probably some changes which need to be made after some large PR's are merged, and probably also some changes to be done to let it work a bit more seamless with the S3 feature.

@lukaspieper
Copy link
Author

Hi,

don't worry, as mentioned in the PR description, I already figured out that there are alternative approaches to solve this issue. I don't mind having a fixed path, assuming we can then easily create a mount from the host to the fixed directory inside the container.

If you would like me to update the PR, please let me know once the other PRs are complete and what the solution should look like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants