Resolve config and temp paths independently of the working directory - #93
Conversation
kbrddestroyer
left a comment
There was a problem hiding this comment.
As commented, I'd suggest storing configs inside project directory. Leave a comment if you have any other options
|
|
||
| def get_main_config_directory() -> Path: | ||
| home_dir = Path(os.path.abspath(os.curdir)) | ||
| home_dir = Path.home() |
There was a problem hiding this comment.
Well, if we're working with home directory I'd rather pick .config/ folder. But as for me, the best way would be to resolve project install directory and work inside it.
Maybe something like Path(__file__).parent() would work here?
| # Create temp folder in project path on Linux as well | ||
| if not os.path.exists(TMP_FOLDER): | ||
| os.mkdir(TMP_FOLDER) | ||
| # Created on demand by whatever writes there, rather than as an import side |
There was a problem hiding this comment.
Fair enough, but I think this comment is not necessary
Three paths in configs/constants.py resolve against the current working directory, so an installed copy only works when run from a source checkout. 1. PATH_TO_DEFAULT_CONFIG was 'src/epomakercontroller/configs/default.json', a path that only exists inside a checkout. From anywhere else, load_main_config() raises FileNotFoundError -- which includes any desktop launcher or systemd service. Now resolved against __file__. 2. get_main_config_directory() used Path(os.path.abspath(os.curdir)), so each working directory got its own config. Where it writes is now selected by USE_XDG_CONFIG_DIR in constants.py: True (the default) uses $XDG_CONFIG_HOME, falling back to ~/.config; False keeps it alongside the installed package. The suite passes either way. 3. constants.py ran os.mkdir at import time, creating a directory in the caller's working directory as a side effect of importing the package. The folder moves under tempfile.gettempdir() and is created by generate_udev_rule(), the only code that writes there. test_constants.py asserted the temp folder existed immediately after import, which only held because of that import-time mkdir. It now checks that the writable paths are absolute, that config lives outside the package when USE_XDG_CONFIG_DIR is set, and that the bundled default config is readable.
554d96c to
010e2d9
Compare
|
Thanks for the review, and for merging #92. Switched to For the user config I've put it behind a switch so you can take either without another round-trip: USE_XDG_CONFIG_DIR = True # False -> alongside the installed packageSuite passes both ways. Defaulted to CI failure was |
Three related paths in
configs/constants.pyresolve against the current working directory, so an installed copy only works when run from a source checkout.1.
PATH_TO_DEFAULT_CONFIGcannot be found after installThat path only exists inside a checkout. From anywhere else:
This affects any desktop launcher or systemd service, where the working directory is the home directory. Now resolved against
__file__, where the file is actually installed.2. The config directory follows the working directory
The variable is named
home_dirbut holdsos.curdir, so each working directory gets its own.epomaker-controller/. 0.0.8 usedPath.home(); restored.3.
constants.pycreates a directory at import timeImporting the package creates a directory in the caller's working directory. Moved under
tempfile.gettempdir(), and created bygenerate_udev_rule()— the only thing that writes there — so behaviour is unchanged.Found while building a GTK front end against 0.0.9; I currently patch these at import to make a systemd user service work.