MycroftAI / mycroft-core

Mycroft Core, the Mycroft Artificial Intelligence platform.
https://mycroft.ai
Apache License 2.0
6.49k stars 1.27k forks source link

XDG `save_config_path` raises `FileExistsError` #3044

Closed krisgesling closed 2 years ago

krisgesling commented 2 years ago

Describe the bug The pyxdg save_config_path ensures that a directory exists before using it. This is done using os.path: https://github.com/takluyver/pyxdg/blob/master/xdg/BaseDirectory.py#L57-L58

Based on the Traceback below I'm wondering if we have items starting in separate threads, that are both checking/creating the XDG paths.

Traceback (most recent call last):
  File "/usr/lib/python3.8/runpy.py", line 185, in _run_module_as_main
    mod_name, mod_spec, code = _get_module_details(mod_name, _Error)
  File "/usr/lib/python3.8/runpy.py", line 111, in _get_module_details
    __import__(pkg_name)
  File "/opt/mycroft/mycroft/__init__.py", line 17, in <module>
    from mycroft.api import Api
  File "/opt/mycroft/mycroft/api/__init__.py", line 23, in <module>
    from mycroft.configuration import Configuration
  File "/opt/mycroft/mycroft/configuration/__init__.py", line 15, in <module>
    from .config import Configuration, LocalConf, RemoteConf
  File "/opt/mycroft/mycroft/configuration/config.py", line 28, in <module>
    from .locations import DEFAULT_CONFIG, USER_CONFIG, OLD_USER_CONFIG
  File "/opt/mycroft/mycroft/configuration/locations.py", line 26, in <module>
    USER_CONFIG = join(xdg.BaseDirectory.save_config_path('mycroft'),
  File "/opt/mycroft/.venv/lib/python3.8/site-packages/xdg/BaseDirectory.py", line 58, in save_config_path
    os.makedirs(path, 0o700)
  File "/usr/lib/python3.8/os.py", line 223, in makedirs
    mkdir(name, mode)
FileExistsError: [Errno 17] File exists: '/home/mycroft/.config/mycroft'

I haven't yet found other projects hitting the same issue.

Expected behavior Create the directory if it doesn't exist, otherwise just use it.

Environment (please complete the following information):

JarbasAl commented 2 years ago

I hit this issue with docker containers and flagged it back then, i recommended to use xdg_data_home instead of save_data_dirs etc.

A string definition should be a string definition, not create paths automatically on import

forslund commented 2 years ago

Sorry for missing / forgetting about that @JarbasAl. That is definitely a better way to handle it.