Config changes per Issue #11 #12
Conversation
Keeping the next diff cleaner.
No code changes, just adding comments and moving code. The diff is bad enough without having to look for a code change too.
Next commits will implement based on comments/discussion from: peterkaminski#11
The encoded JSON needs to be a raw string to prevent backslash quoting explosion
Added primitive tracing to help when things do work as expected. Very preliminary implmentation. ENV VARs are a quick way to set values before the command line is parsed but aren't as user friendly.
This also involved tracking the Obsidian Root dirctory since it is no longer a parameter or env var to the script
Also, hoist the code for version and config printing to above where the configs are loaded. They don't depend on it, and this makes it easier to bootstrap a custom configuration.
…idian config is loading from.
osm.py
Outdated
| @@ -401,6 +417,8 @@ def main(): | |||
| print("Error: Cannot locate the 'diff' command, aborting.") | |||
| exit(-1) | |||
|
|
|||
| load_osm_config() | |||
There was a problem hiding this comment.
Yeah, I know, I forgot the args.config parameter here, it will be fixed in a later commit :-)
Helps us to be more careful about using the config. Usages to follow in the next commits.
Next commits will implement finding the files in a specific vault.
osm.py
Outdated
| @@ -492,6 +538,7 @@ def main(): | |||
|
|
|||
| try: | |||
| vault_paths = get_vault_paths() | |||
| directives = list(get_processing_directives()) | |||
There was a problem hiding this comment.
I was on the fence about where in the code to do this as it is a defacto check on the configuration file even if no file operations are going to be performed.
On the one hand, validating early is good.
On the the other hand, validating when it won't be used (say for --list) could make it harder for someone to incrementally create/tweak their config...
There was a problem hiding this comment.
Either way is fine, of course - I can make UX arguments either way, and so for me, the UX arguments are kind of a wash.
Tiebreaker might be simplicity / readability of the code itself, and I think that argues for validating early.
There was a problem hiding this comment.
Yeah, I think I am ok with early too until a compelling argument comes in for postponing.
Probably needs more work too.
It's much easier on the human brain to see them in sorted order esp when looking at diffs.
Sure, you could use `--diff-to` and try to sort it out, but as you are adjusting your files_to_copy list, it would be nice to see how it would pull files from a specific vault without the other output
|
Putting pen down as of commit 19 to sort out the backups thing. |
Was left over from when there were two separate backup action functions.
| ] | ||
| ], | ||
| "backup": { | ||
| "archive_format_priority_list": ["zip", "gztar", "bztar", "xztar", "tar"], |
There was a problem hiding this comment.
@peterkaminski the idea here is that since any one format might not be available, better to have a list of preferred formats, and work left to right until make_archive works.
I'm open to better names for the keys/values here.
|
Whoa, has it been two weeks! Crikey! |
Me too! 🙂 |
Remove the leading dash so that it can be used as a prefix. Match the ISO string anywhere, prefix, suffix, or embedded.
Also backup before doing an update.
NOTE: update hasn't been fixed yet,
only the backup code is working at this point
|
@peterkaminski - take a look at the latest two commits. |
osm.py
Outdated
| try: | ||
| backup_file = shutil.make_archive(backup_file, backup_format, root_dir=backup_from_dir) | ||
| if backup_file: | ||
| print("Backup made:", backup_file) |
There was a problem hiding this comment.
If we like how this works, I'll update to have versbose and dry-run implemented. The trick with dry-run though is that until we try to call make_archive we don't know for sure sure sure if that format is actually supported.
There is https://docs.python.org/3/library/shutil.html#shutil.get_archive_formats but I am not sure if those are guaranteed to work, or if they are just potentially available...
There was a problem hiding this comment.
shutil.make_archive() takes a dry_run boolean. ;-)
There was a problem hiding this comment.
D'oh! So it does! I think I knew that when I first found it, but then forgot, thanks!
@dgou, looks good to me. |
| @@ -536,7 +536,7 @@ def diff_settings(dest, src): | |||
|
|
|||
|
|
|||
| def backup_vault(vault_path): | |||
| print('#', vault_path) | |||
| print(f'# Backing up settings for: {vault_path}') | |||
There was a problem hiding this comment.
I wasn't sure about keeping this as print vs verbose but when I was testing it I liked seeing that it was doing something and where, but I'm open to making this verbose instead.
@peterkaminski
Submitting this as a pull request, but still WIP.
All the config file loading stuff is done.
The copy/diff part is NOT done because I have questions about how backups work when we start cherry-picking files deeper in the vault configuration than the top level directories.
To requests: