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

Cache handling #34

Open
darkdragon-001 opened this issue May 25, 2020 · 5 comments
Open

Cache handling #34

darkdragon-001 opened this issue May 25, 2020 · 5 comments

Comments

@darkdragon-001
Copy link

darkdragon-001 commented May 25, 2020

Optimized cache management is important to keep cloud storage costs under control.

I would like to use --cache-dir for all operations. Having this built-in makes sure I don't forget it for any command.

The other options like --with-cache/--no-cache can be specified via -- --option if you don't want to add official support.


Furthermore, behavior different from restic default should be documented:

if os.geteuid() == 0: # pragma: no cover; if user is root, we just use system cache
os.environ["XDG_CACHE_HOME"] = "/var/cache"
elif not (os.environ.get("HOME") or os.environ.get("XDG_CACHE_HOME")):
os.environ["XDG_CACHE_HOME"] = "/var/cache"

@andreasnuesslein
Copy link
Member

I thank you for all your comments. You clearly know what you're talking about.

Would you mind create a PR or two for some of the suggestions you're making?

Cheers

@darkdragon-001
Copy link
Author

How should it go in the config file?

@andreasnuesslein
Copy link
Member

andreasnuesslein commented May 28, 2020

So according to restic, it should work to just set XDG_CACHE_DIR because all restic commands seem to honor that. However I know I put in logic that is thwarting that at the moment. So I suggest we throw out the custom if os.geteuid() == 0: logic. Next we recommend setting XDG_CACHE_DIR in [environment] in the docs and lastly print a warning on running runrestic when no explicit XDG_CACHE_DIR is set to let the user know what runrestic assumes then.
i.e.: [warning] You have not explicitily set XDG_CACHE_DIR. Falling back to /var/cache/

@darkdragon-001
Copy link
Author

darkdragon-001 commented May 29, 2020

I just noticed RESTIC_CACHE_DIR environment variable is also supported. So this can just be used in the config environment section if the user wants to use it! Therefore, I would just remove this code block...

I suggest to add a warning right now that the behavior will change in a future version when one of the quoted code blocks is executed. And then just remove this code in the next bigger update.

I added an issue to get a complete list of environment variables in restic/restic#2763.

@darkdragon-001
Copy link
Author

You can pass env to subprocess.Popen().

I suggest the following:

env = { name: os.environ[name] for name in ['HOME','XDG_CACHE_DIR','TMPDIR'] if name in os.environ }
env = { **env, **config['environment'] }

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

No branches or pull requests

2 participants