Implement importer #8
Conversation
| finally: | ||
| log.debug(' Moving {} -> {}'.format(import_path, rsnapshot['path'])) | ||
| import_path.rename(snapshot_original_path) | ||
| import_journal.unlink() |
There was a problem hiding this comment.
Probably want an option to not do that. Trading "doesn't fiddle with the import source" and "it's fukkken fast!"
| for rsnapshot in get_snapshots(args.rsnapshot_root): | ||
| print(rsnapshot) | ||
| timestamp = rsnapshot['timestamp'].replace(microsecond=0) |
There was a problem hiding this comment.
Borg should probably be able to accept --timestamps with microsecond resolution.
| @@ -46,7 +46,7 @@ | |||
| zip_safe=False, | |||
| entry_points={ | |||
| 'console_scripts': [ | |||
| 'borg-import = borg_import.main:main', | |||
| 'borg-import = borg_import.main:below_main', | |||
There was a problem hiding this comment.
well it's physically correct - below_main is literally below main - but I'm mostly referring to the C runtime meaning here, the functions actually running before main.
but with the meanwhile refactorings it isn't needed anymore
| @@ -4,7 +4,7 @@ | |||
|
|
|||
| def discover(root, depth): | |||
| """ | |||
| recurse starting from <root> path and yield relative dir pathes with wanted <depth>. | |||
| recurse starting from <root> path and yield relative dir paths with wanted <depth>. | |||
| @@ -1,18 +1,19 @@ | |||
| import os | |||
| from datetime import datetime | |||
| from pathlib import Path | |||
There was a problem hiding this comment.
seems rather like general refactoring than "implement importer".
| def borg_import(args, archive_name, path, timestamp=None): | ||
| borg_cmdline = ['borg', 'create'] | ||
| if timestamp: | ||
| borg_cmdline += '--timestamp', timestamp.isoformat() |
There was a problem hiding this comment.
[1,2]+(3,4)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: can only concatenate list (not "tuple") to list
There was a problem hiding this comment.
Doesn't apply to += / extend
|
|
||
| log.debug('Borg command line: %r', borg_cmdline) | ||
| log.debug('Borg working directory: %s', path) | ||
| subprocess.check_call(borg_cmdline, cwd=str(path)) |
There was a problem hiding this comment.
that will raise even on warnings. which is maybe even ok for a converter.
| parser = argparse.ArgumentParser() | ||
| parser.add_argument("--rsnapshot-root", help="Path to rsnapshot root directory") | ||
| if not shutil.which('borg'): | ||
| print('Borg does not seem to be installed. Please install Borg first.') |
There was a problem hiding this comment.
Maybe rather:
"The 'borg' command can't be found in the PATH. Please correctly install borgbackup first."
|
|
||
| common_group.add_argument("--create-options", "-o", | ||
| help="Additional borg create options " | ||
| "(note: Use -o=\"--foo --bar\" syntax to avoid parser confusion).") |
|
|
||
| subparsers = parser.add_subparsers() | ||
|
|
||
| for importer in (importer() for importer in Importer.__subclasses__()): |
There was a problem hiding this comment.
why not simpler like:
for importer_class in Importer.__subclasses__():
importer = importer_class()
...
| if args.rsnapshot_root: | ||
| for rsnapshot in get_snapshots(args.rsnapshot_root): | ||
| print(rsnapshot) | ||
| if 'function' not in args: |
There was a problem hiding this comment.
hmm, is args also a dict so no hasattr() is needed?
There was a problem hiding this comment.
argparse.Namespace is special that way
|
|
||
|
|
||
| def below_main(): | ||
| # We Don't Go To Ravenholm |
|
btw. I think if we're really (ever) add another importer then it would probably be better to just make them different commands altogether, |
| except subprocess.CalledProcessError as cpe: | ||
| if cpe.returncode != 1: | ||
| raise | ||
| log.debug('Borg exited with a warning (being quiet about it since Borg spoke already)') |
There was a problem hiding this comment.
somewhere at the beginning, we need to add "all your data will get assimilated now...". :-)
|
One importer I definitely want to add is for more self-made rsync+hardlinks scripts (like I used personally for a long time). Quite similar to what we have, but not quite. See #7. |
|
OK, I didn't try it, but assuming it works: collapse some of the commits, merge? |
|
It at least works for a basic configuration with two backup sets and about two dozen snapshots across two "generations" (alpha/beta/...). Should be good (for now). Wondering if/how we want to properly test this. I thought... go the easy way, just add a full test case to the git repo? (But we have time to figure that out) |
|
maybe we could have a tar with test input data for the different source types an unpack it for the tests. |
|
=> #9 |
|
I don't have write access, you'll need to merge yourself ;) |
No description provided.