WIP: Add locking of borgmatic config file #254
|
@ -1,4 +1,5 @@
|
||||||
import collections
|
import collections
|
||||||
|
import fcntl
|
||||||
import json
|
import json
|
||||||
import logging
|
import logging
|
||||||
import os
|
import os
|
||||||
|
@ -54,6 +55,17 @@ def run_configuration(config_filename, config, arguments):
|
||||||
error_repository = ''
|
error_repository = ''
|
||||||
prune_create_or_check = {'prune', 'create', 'check'}.intersection(arguments)
|
prune_create_or_check = {'prune', 'create', 'check'}.intersection(arguments)
|
||||||
|
|
||||||
|
|||||||
|
if location.get("lock_client", False):
|
||||||
witten
commented
Make sure you run tests with Make sure you run tests with `tox`, as the code formatter will probably complain about this. To have it reformat things for you: `tox -e black` is all you need to do.
|
|||||||
|
lock_f = open(config_filename)
|
||||||
|
try:
|
||||||
|
fcntl.flock(lock_f.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB)
|
||||||
|
except IOError as error:
|
||||||
|
encountered_error = error
|
||||||
|
yield from make_error_log_records(
|
||||||
|
'{}: Failed to acquire lock'.format(config_filename), error
|
||||||
|
)
|
||||||
|
|
||||||
|
if not encountered_error:
|
||||||
try:
|
try:
|
||||||
witten
commented
These can be combined into a single These can be combined into a single `if` statement.
|
|||||||
if prune_create_or_check:
|
if prune_create_or_check:
|
||||||
dispatch.call_hooks(
|
dispatch.call_hooks(
|
||||||
|
|
|
@ -32,6 +32,10 @@ map:
|
||||||
type: bool
|
type: bool
|
||||||
desc: Stay in same file system (do not cross mount points). Defaults to false.
|
desc: Stay in same file system (do not cross mount points). Defaults to false.
|
||||||
example: true
|
example: true
|
||||||
|
lock_client:
|
||||||
|
type: bool
|
||||||
|
desc: Lock config when running borgmatic to prevent multiple instances from running simultaneously. Defaults to false.
|
||||||
witten
commented
You could add: ". Defaults to false." here. See other descriptions for an example. You could add: ". Defaults to false." here. See other descriptions for an example.
|
|||||||
|
example: true
|
||||||
witten
commented
The fact that it's locking the configuration file might just be an implementation detail as far as the user is concerned. What about calling this something like The fact that it's locking the configuration file might just be an implementation detail as far as the user is concerned. What about calling this something like `lock_client`, `lock_borgmatic`, etc? I don't feel strongly about this in any case.
drewkett
commented
Those sound better. I’ll fix it when I get the chance Those sound better. I’ll fix it when I get the chance
drewkett
commented
I renamed it to I renamed it to `lock_client` and left it as a bool for now. I didn't see immediately the best way to do it with the config (and validation), but if you want to keep options open switching this to something like `lock: "config"` so that then you could add a `lock: "everything` option later would be an option.
drewkett
commented
I also wasn't sure if the example line is supposed to be the default value or not. I also wasn't sure if the example line is supposed to be the default value or not.
witten
commented
A bool is fine for now, as far as I'm concerned.
A bool is fine for now, as far as I'm concerned.
`example:` is not usually the default value. In fact it's often the *opposite* of the default value in that it's demonstrating how you would override the default if you are so inclined. So `example: true` sounds right to me here, given that I'm assuming the default is `false` (don't lock).
|
|||||||
numeric_owner:
|
numeric_owner:
|
||||||
type: bool
|
type: bool
|
||||||
desc: Only store/extract numeric user and group identifiers. Defaults to false.
|
desc: Only store/extract numeric user and group identifiers. Defaults to false.
|
||||||
|
|
Given that this is in
run_configuration()
now, you can just:... and then this would go through the rest of the borgmatic logging/error/summary machinery.
Another option would be this:
Which would fall through and trigger the
on_error
hook, if that's what you want to do in this case.