check_last parameter data type validation #33

New Issue

import_bot · 2018-01-05T05:34:20Z

import_bot commented

2018-01-05 05:34:20 +00:00

Preconditions

Versions:

borgmatic 1.1.0
borgbackup 1.0.10

Convert an existing config file (from v1.0) with upgrade-borgmatic-config that has the check_last configuration option defined.
Example:

[consistency]
# Space-separated list of consistency checks to run: "repository", "archives",
# or both. Defaults to both. Set to "disabled" to disable all consistency
# checks. See borgbackup.readthedocs.org/en/stable/usage.html#borg-check
# for details.
checks: archives

# Restrict the number of checked archives to the last n.
check_last: 2

Is converted to

# Consistency checks to run after backups. See
# borgbackup.readthedocs.org/en/stable/usage.html#borg-check for details.
consistency:
    # List of consistency checks to run: "repository", "archives", or both. Defaults
    # to both. Set to "disabled" to disable all consistency checks. See
    # borgbackup.readthedocs.org/en/stable/usage.html#borg-check for details.
    checks:
        - archives

    # Restrict the number of checked archives to the last n.
    check_last: '2'

Steps to Reproduce

Run Borgmatic with borgmatic -v 2 -c /etc/borgmatic/config.yaml

Result

validation.invalid
 --- All found errors ---
["Value '2' is not of type 'int'. Path: '/consistency/check_last'"]
Errors found but will not raise exception...
('/etc/borgmatic/config.yaml', ["Value '2' is not of type 'int'. Path: '/consistency/check_last'"])

The number was converted to a string and the validation complains that an int is expected.

However, when I change the value from check_last: '2' to check_last: 2, this error is thrown by borgmatic when running borgmatic -v 2 -c /etc/borgmatic/config.yaml again:

[...]

Traceback (most recent call last):
  File "/bin/borgmatic", line 11, in <module>
    sys.exit(main())
  File "/usr/lib/python3.4/site-packages/borgmatic/commands/borgmatic.py", line 63, in main
    borg.check_archives(args.verbosity, repository, consistency, remote_path=remote_path)
  File "/usr/lib/python3.4/site-packages/borgmatic/borg.py", line 203, in check_archives
    subprocess.check_call(full_command, stdout=stdout, stderr=subprocess.STDOUT)
  File "/usr/lib64/python3.4/subprocess.py", line 553, in check_call
    retcode = call(*popenargs, **kwargs)
  File "/usr/lib64/python3.4/subprocess.py", line 534, in call
    with Popen(*popenargs, **kwargs) as p:
  File "/usr/lib64/python3.4/subprocess.py", line 856, in __init__
    restore_signals, start_new_session)
  File "/usr/lib64/python3.4/subprocess.py", line 1398, in _execute_child
    restore_signals, start_new_session, preexec_fn)
TypeError: Can't convert 'int' object to str implicitly

So it appears the string value is the expected one and the config file validation is wrong.

The backup functionality is not impacted, only the consistency checks (thus setting the severity to 'Important'.

Additional note:
generate-borgmatic-config also generates a config file with an int:

# Consistency checks to run after backups. See
# borgbackup.readthedocs.org/en/stable/usage.html#borg-check for details.
consistency:
    # List of consistency checks to run: "repository", "archives", or both. Defaults
    # to both. Set to "disabled" to disable all consistency checks. See
    # borgbackup.readthedocs.org/en/stable/usage.html#borg-check for details.
    checks:
        - repository
        - archives

    # Restrict the number of checked archives to the last n.
    check_last: 3

Imported from Taiga issue 32 (done). Created on 2017-07-24T11:00:02+0000 by Simon Dellenbach.

### Preconditions Versions: * borgmatic 1.1.0 * borgbackup 1.0.10 Convert an existing config file (from v1.0) with `upgrade-borgmatic-config` that has the `check_last` configuration option defined. Example: ``` [consistency] # Space-separated list of consistency checks to run: "repository", "archives", # or both. Defaults to both. Set to "disabled" to disable all consistency # checks. See borgbackup.readthedocs.org/en/stable/usage.html#borg-check # for details. checks: archives # Restrict the number of checked archives to the last n. check_last: 2 ``` Is converted to ``` # Consistency checks to run after backups. See # borgbackup.readthedocs.org/en/stable/usage.html#borg-check for details. consistency: # List of consistency checks to run: "repository", "archives", or both. Defaults # to both. Set to "disabled" to disable all consistency checks. See # borgbackup.readthedocs.org/en/stable/usage.html#borg-check for details. checks: - archives # Restrict the number of checked archives to the last n. check_last: '2' ``` ### Steps to Reproduce 1. Run Borgmatic with `borgmatic -v 2 -c /etc/borgmatic/config.yaml` ### Result ``` validation.invalid --- All found errors --- ["Value '2' is not of type 'int'. Path: '/consistency/check_last'"] Errors found but will not raise exception... ('/etc/borgmatic/config.yaml', ["Value '2' is not of type 'int'. Path: '/consistency/check_last'"]) ``` The number was converted to a string and the validation complains that an int is expected. However, when I change the value from `check_last: '2'` to `check_last: 2`, this error is thrown by borgmatic when running `borgmatic -v 2 -c /etc/borgmatic/config.yaml` again: ``` [...] Traceback (most recent call last): File "/bin/borgmatic", line 11, in <module> sys.exit(main()) File "/usr/lib/python3.4/site-packages/borgmatic/commands/borgmatic.py", line 63, in main borg.check_archives(args.verbosity, repository, consistency, remote_path=remote_path) File "/usr/lib/python3.4/site-packages/borgmatic/borg.py", line 203, in check_archives subprocess.check_call(full_command, stdout=stdout, stderr=subprocess.STDOUT) File "/usr/lib64/python3.4/subprocess.py", line 553, in check_call retcode = call(*popenargs, **kwargs) File "/usr/lib64/python3.4/subprocess.py", line 534, in call with Popen(*popenargs, **kwargs) as p: File "/usr/lib64/python3.4/subprocess.py", line 856, in __init__ restore_signals, start_new_session) File "/usr/lib64/python3.4/subprocess.py", line 1398, in _execute_child restore_signals, start_new_session, preexec_fn) TypeError: Can't convert 'int' object to str implicitly ``` So it appears the string value is the expected one and the config file validation is wrong. The backup functionality is not impacted, only the consistency checks (thus setting the severity to 'Important'. Additional note: `generate-borgmatic-config` also generates a config file with an int: ``` # Consistency checks to run after backups. See # borgbackup.readthedocs.org/en/stable/usage.html#borg-check for details. consistency: # List of consistency checks to run: "repository", "archives", or both. Defaults # to both. Set to "disabled" to disable all consistency checks. See # borgbackup.readthedocs.org/en/stable/usage.html#borg-check for details. checks: - repository - archives # Restrict the number of checked archives to the last n. check_last: 3 ``` --- Imported from Taiga issue 32 (done). Created on 2017-07-24T11:00:02+0000 by Simon Dellenbach.

import_bot closed this issue

2018-01-05 05:34:20 +00:00

import_bot added the

bug

label 2018-01-05 05:34:21 +00:00

import_bot commented

2018-01-05 05:34:21 +00:00

Thanks for reporting! I've fixed the portion that upgrades the config incorrectly. However, I'll still need to fix the other problem you found: We need to convert everything to strings before passing values to subprocess and Borg.

Comment on 2017-07-24T15:45:52+0000 by Dan Helfman.

Thanks for reporting! I've fixed the portion that upgrades the config incorrectly. However, I'll still need to fix the other problem you found: We need to convert everything to strings before passing values to subprocess and Borg. --- Comment on 2017-07-24T15:45:52+0000 by Dan Helfman.

import_bot commented

2018-01-05 05:34:21 +00:00

Okay, this should be fixed now in borgmatic 1.1.2 (just released). Thanks again for reporting it with such detail about the repro case.

Comment on 2017-07-25T02:32:14+0000 by Dan Helfman.

Okay, this should be fixed now in borgmatic 1.1.2 (just released). Thanks again for reporting it with such detail about the repro case. --- Comment on 2017-07-25T02:32:14+0000 by Dan Helfman.

import_bot commented

2018-01-05 05:34:22 +00:00

Yes, it works fine again in 1.1.2. I've also re-run the upgrade of my old config file and verified that the value is correct.
Thank you very much for the quick fix!

Comment on 2017-07-25T15:48:43+0000 by Simon Dellenbach.

Yes, it works fine again in 1.1.2\. I've also re-run the upgrade of my old config file and verified that the value is correct. Thank you very much for the quick fix! --- Comment on 2017-07-25T15:48:43+0000 by Simon Dellenbach.

Sign in to join this conversation.