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

v3 breaking changes #80

Open
12 tasks
m90 opened this issue Mar 6, 2022 · 9 comments
Open
12 tasks

v3 breaking changes #80

m90 opened this issue Mar 6, 2022 · 9 comments
Labels

Comments

@m90
Copy link
Member

m90 commented Mar 6, 2022

This issue exists to track possible breaking changes that would make sense in a v3 (so I don't forget about them). This does not mean a v3 is going to happen soon. It could even be it never happens.


Remove EMAIL_* config

As all notifications are now sent using shoutrrr, the EMAIL_* interface is obsolete and should be removed.

  • Remove configuration shim
  • Remove documentation

Remove BACKUP_FROM_SNAPSHOT

The functionality of BACKUP_FROM_SNAPSHOT can be achieved by using and exec-pre and exec-post command.

  • Remove feature
  • Remove documentation

Default to expanding variables in BACKUP_FILENAME

The option to do this has been introduced later on and false was chosen as a default in order to not introduce a breaking change. true would be the better and more helpful default.

  • Set default to true
  • Update documentation

Also consider doing this for all other configuration values.

Rename BACKUP_SOURCES

BACKUP_SOURCES points to a single location. It should be called BACKUP_SOURCE (just like BACKUP_ARCHIVE).

  • Rename to BACKUP_SOURCE
  • Update documentation

Remove exec-[pre|post] labels

The labels have more granularity now.

  • Remove all handling of exec-pre and exec-post
  • Update documentation

Remove BACKUP_STOP_CONTAINER_LABEL

The BACKUP_STOP_CONTAINER_LABEL setting has been renamed: https://offen.github.io/docker-volume-backup/how-tos/replace-deprecated-backup-stop-container-label.html

  • Remove setting
  • Remove documentation
@rpatel3001
Copy link
Contributor

consider rclone as a universal backend per #65? Ideally combinable with multiple sources and multiple schedules.

@m90
Copy link
Member Author

m90 commented Mar 22, 2022

Ideally combinable with multiple sources and multiple schedules.

Could you elaborate on how this would be different from what is already possible?

@rpatel3001
Copy link
Contributor

I don't use the config file directory so I didn't know how it works but after looking at it I think it should combine just fine if every environment variable can be set individually for each run. The only difference would be another var with a list of rclone remote:folder targets to upload to. I'd also like a var that controls syncing all archives or just the symlinked latest one (I have a space constrained VPS that I sync just my latest to).

It would also be nice if the backup executable was able to be called multiple time in parallel to allow the same cron expression in multiple config files. Or maybe have backup configs leave out the cron expression and then have separate schedule configs, each with a cron expression and a list of backup configs to run sequentially. That might complicate setting up the container too much.

@m90
Copy link
Member Author

m90 commented Mar 23, 2022

It would also be nice if the backup executable was able to be called multiple time in parallel

As the packaging format of this tool is a Docker image, I'm not entirely sure if getting rid of the exclusive lock (which probably is possible, albeit makes things more complicated) is worth it. If you really need to, you could still spawn multiple containers from that image which run in parallel just fine.

If the backup command were to be unboxed and distributed as a binary, other constraints would apply here, but up until then I feel it's ok to keep the lock and the simplicity it brings.

@rpatel3001
Copy link
Contributor

What is it that requires the lock? Or, instead of refactoring to not require a lock, could you check the lock in a loop until some timeout, to support identical cron expressions?

@m90
Copy link
Member Author

m90 commented Mar 23, 2022

The lock is there for two reasons mostly:

  • making sure certain filesystem paths are available for exclusive use by the script (this is what could be refactored)
  • making sure the behavior around starting/stopping/inspecting Docker containers is predictable (this is mostly there to reduce confusion in users)

I think your idea of waiting for the lock to become available instead of failing hard is a pretty good one though, so that should be a workable solution that allows reuse of cron schedules while still meeting above requirements.

@m90
Copy link
Member Author

m90 commented Mar 24, 2022

I added support for identical cron expressions in #87 - I'll have another look at this myself tomorrow or so, but in case you have any feedback on that @rpatel3001 feel free to chime in.

@rpatel3001
Copy link
Contributor

looks good to me

@m90
Copy link
Member Author

m90 commented Mar 25, 2022

Support for identical and overlapping cron schedules in now supported in v2.15.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants