Dikootje/sqlfluff
1
0
Fork 0
forked from repo-mirrors/sqlfluff
Code Pull Requests Activity

Switch from appdirs to platformdirs (#6399)

Browse Source 
Co-authored-by: Alan Cruickshank <alan@Alans-MacBook-Pro.local>
Co-authored-by: Alan Cruickshank <alan+git@a14k.co.uk>
This commit is contained in:
Alan Cruickshank
2024-11-21 15:02:09 +00:00
committed by GitHub
parent 80f4fc2d3b
commit 4dc390f077
6 changed files with 174 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
.pre-commit-config.yaml
View File

@@ -45,7 +45,6 @@ repos:
[
types-toml,
types-chardet,
types-appdirs,
types-colorama,
types-pyyaml,
types-regex,
@@ -59,6 +58,7 @@ repos:
pathspec,
pytest, # and by extension... pluggy
click,
platformdirs
]
files: ^src/sqlfluff/.*
# The mypy pre-commit hook by default sets a few arguments that we don't normally

2
pyproject.toml
View File

@@ -67,7 +67,7 @@ keywords = [
]
dependencies = [
# Used for finding os-specific application config dirs
"appdirs",
"platformdirs",
# To get the encoding of files.
"chardet",
"click",

78
src/sqlfluff/core/config/loader.py
View File

@@ -17,12 +17,15 @@ except ImportError: # pragma: no cover
import logging
import os
import os.path
import sys
from pathlib import Path
from typing import (
Optional,
)
import appdirs
import platformdirs
import platformdirs.macos
import platformdirs.unix
from sqlfluff.core.config.file import (
cache,
@@ -55,22 +58,50 @@ ALLOWABLE_LAYOUT_CONFIG_KEYS = (
)
def _get_user_config_dir_path() -> str:
def _get_user_config_dir_path(sys_platform: str) -> str:
"""Get the user config dir for this system.
Args:
sys_platform (str): The result of ``sys.platform()``. Provided
as an argument here for ease of testing. In normal usage
it should only be called with ``sys.platform()``. This
argument only applies to switching between linux and macos.
Win32 detection still uses the underlying ``sys.platform()``
methods.
"""
appname = "sqlfluff"
appauthor = "sqlfluff"
# On Mac OSX follow Linux XDG base dirs
# First try the default SQLFluff specific cross-platform config path.
cross_platform_path = os.path.expanduser("~/.config/sqlfluff")
if os.path.exists(cross_platform_path):
return cross_platform_path
# Then try the platform specific paths, for MacOS, we check
# the unix variant first to preferentially use the XDG config path if set.
# https://github.com/sqlfluff/sqlfluff/issues/889
user_config_dir_path = os.path.expanduser("~/.config/sqlfluff")
if appdirs.system == "darwin":
appdirs.system = "linux2"
user_config_dir_path = appdirs.user_config_dir(appname, appauthor)
appdirs.system = "darwin"
if not os.path.exists(user_config_dir_path):
user_config_dir_path = appdirs.user_config_dir(appname, appauthor)
return user_config_dir_path
if sys_platform == "darwin":
unix_config_path = platformdirs.unix.Unix(
appname=appname, appauthor=appauthor
).user_config_dir
if os.path.exists(os.path.expanduser(unix_config_path)):
return unix_config_path
# Technically we could just delegate to the generic `user_config_dir`
# method, but for testing it's convenient to explicitly call the macos
# methods here.
return platformdirs.macos.MacOS(
appname=appname, appauthor=appauthor
).user_config_dir
# NOTE: We could delegate to the generic `user_config_dir` method here,
# but for testing it's convenient to explicitly call the linux methods.
elif sys_platform == "linux":
return platformdirs.unix.Unix(
appname=appname, appauthor=appauthor
).user_config_dir
# Defer to the self-detecting paths.
# NOTE: On Windows this means that the `sys_platform` argument is not
# applied.
return platformdirs.user_config_dir(appname, appauthor)
def load_config_file(
@@ -218,7 +249,7 @@ def load_config_at_path(path: str) -> ConfigMappingType:
def _load_user_appdir_config() -> ConfigMappingType:
"""Load the config from the user's OS specific appdir config directory."""
user_config_dir_path = _get_user_config_dir_path()
user_config_dir_path = _get_user_config_dir_path(sys.platform)
if os.path.exists(user_config_dir_path):
return load_config_at_path(user_config_dir_path)
else:
@@ -283,16 +314,19 @@ def load_config_up_to_path(
config_paths = iter_intermediate_paths(Path(path).absolute(), Path.cwd())
config_stack = [load_config_at_path(str(p.resolve())) for p in config_paths]
# 4) Extra config paths
if not extra_config_path:
# 4) Extra config paths.
# When calling `load_config_file_as_dict` we resolve the path first so that caching
# is more efficient.
extra_config = {}
else:
if not os.path.exists(extra_config_path):
raise SQLFluffUserError(
f"Extra config '{extra_config_path}' does not exist."
if extra_config_path:
try:
extra_config = load_config_file_as_dict(
str(Path(extra_config_path).resolve())
)
except FileNotFoundError:
raise SQLFluffUserError(
f"Extra config path '{extra_config_path}' does not exist."
)
# Resolve the path so that the caching is accurate.
extra_config = load_config_file_as_dict(str(Path(extra_config_path).resolve()))
return nested_combine(
user_appdir_config,

4
test/cli/commands_test.py
View File

@@ -228,8 +228,8 @@ def test__cli__command_extra_config_fail():
],
],
assert_output_contains=(
"Extra config 'test/fixtures/cli/extra_configs/.sqlfluffsdfdfdfsfd' does "
"not exist."
"Extra config path 'test/fixtures/cli/extra_configs/.sqlfluffsdfdfdfsfd' "
"does not exist."
),
)

128
test/core/config/loader_test.py
View File

@@ -5,7 +5,6 @@ import sys
from contextlib import contextmanager
from unittest.mock import call, patch
import appdirs
import pytest
from sqlfluff.core import FluffConfig
@@ -19,6 +18,7 @@ from sqlfluff.core.config.loader import (
_get_user_config_dir_path,
_load_user_appdir_config,
)
from sqlfluff.core.errors import SQLFluffUserError
config_a = {
"core": {"testing_val": "foobar", "testing_int": 4, "dialect": "mysql"},
@@ -59,21 +59,43 @@ def test__config__load_file_f():
assert cfg == config_a
def test__config__load_file_missing_extra():
"""Test loading config from a file path if extra path is not found."""
with pytest.raises(SQLFluffUserError):
load_config_up_to_path(
os.path.join("test", "fixtures", "config", "inheritance_a", "testing.sql"),
extra_config_path="non/existent/path",
)
def test__config__load_nested():
"""Test nested overwrite and order of precedence of config files."""
cfg = load_config_up_to_path(
os.path.join(
"test", "fixtures", "config", "inheritance_a", "nested", "blah.sql"
)
),
extra_config_path=os.path.join(
"test",
"fixtures",
"config",
"inheritance_a",
"extra",
"this_can_have_any_name.cfg",
),
)
assert cfg == {
"core": {
# Outer .sqlfluff defines dialect & testing_val and not overridden.
"dialect": "mysql",
"testing_val": "foobar",
# tesing_int is defined in many. Inner pyproject.toml takes precedence.
"testing_int": 1,
# testing_bar is defined only in setup.cfg
"testing_bar": 7.698,
},
"bar": {"foo": "foobar"},
# bar is defined in a few, but the extra_config takes precedence.
"bar": {"foo": "foobarextra"},
# fnarr is defined in a few. Inner tox.ini takes precedence.
"fnarr": {"fnarr": {"foo": "foobar"}},
}
@@ -158,37 +180,107 @@ def test__config__load_placeholder_cfg():
@patch("os.path.exists")
@patch("os.listdir")
@pytest.mark.skipif(sys.platform == "win32", reason="Not applicable on Windows")
def test__config__load_user_appdir_config(
mock_listdir, mock_path_exists, mock_xdg_home
@pytest.mark.parametrize(
"sys_platform,xdg_exists,default_exists,resolved_config_path,paths_checked",
[
# On linux, if the default path exists, it should be the only path we check
# and the chosen config path.
("linux", True, True, "~/.config/sqlfluff", ["~/.config/sqlfluff"]),
# On linux, if the default path doesn't exist, then (because for this
# test case we set XDG_CONFIG_HOME) it will check the default path
# but then on finding it to not exist it will then try the XDG path.
# In this case, neither actually exist and so what matters is that both
# are either checked or used - rather than one in particular being the
# end result.
(
"linux",
False,
False,
"~/.config/my/special/path/sqlfluff",
["~/.config/sqlfluff"],
),
# On MacOS, if the default config path and the XDG path don't exist, then
# we should resolve config to the default MacOS config path.
(
"darwin",
False,
False,
"~/Library/Application Support/sqlfluff",
["~/.config/sqlfluff", "~/.config/my/special/path/sqlfluff"],
),
# However, if XDG_CONFIG_HOME is set, and the path exists then that should
# be resolved _ahead of_ the default MacOS config path (as demonstrated
# by us not checking the presence of that path in the process).
# https://github.com/sqlfluff/sqlfluff/issues/889
(
"darwin",
True,
False,
"~/.config/my/special/path/sqlfluff",
["~/.config/sqlfluff", "~/.config/my/special/path/sqlfluff"],
),
],
)
def test__config__get_user_config_dir_path(
mock_listdir,
mock_path_exists,
mock_xdg_home,
sys_platform,
xdg_exists,
default_exists,
resolved_config_path,
paths_checked,
):
"""Test loading config from user appdir."""
xdg_home = os.environ.get("XDG_CONFIG_HOME")
assert xdg_home, "XDG HOME should be set by the mock. Something has gone wrong."
xdg_config_path = xdg_home + "/sqlfluff"
def path_exists(x):
if x == os.path.expanduser("~/.config/sqlfluff"):
def path_exists(check_path):
"""Patch for os.path.exists which depends on test parameters.
Returns:
True, unless `default_exists` is `False` and the path passed to
the function is the default config path, or unless `xdg_exists`
is `False` and the path passed is the XDG config path.
"""
resolved_path = os.path.expanduser(check_path)
if (
resolved_path == os.path.expanduser("~/.config/sqlfluff")
and not default_exists
):
return False
if x == xdg_config_path:
if resolved_path == os.path.expanduser(xdg_config_path) and not xdg_exists:
return False
else:
return True
mock_path_exists.side_effect = path_exists
with patch.object(appdirs, attribute="system", new="darwin"):
resolved_path = _get_user_config_dir_path()
_load_user_appdir_config()
assert resolved_path == os.path.expanduser("~/Library/Application Support/sqlfluff")
# Get the config path as though we are on macOS.
resolved_path = _get_user_config_dir_path(sys_platform)
assert os.path.expanduser(resolved_path) == os.path.expanduser(resolved_config_path)
mock_path_exists.assert_has_calls(
[
call(xdg_config_path),
call(os.path.expanduser("~/Library/Application Support/sqlfluff")),
]
[call(os.path.expanduser(path)) for path in paths_checked]
)
@patch("os.path.exists")
@patch("sqlfluff.core.config.loader.load_config_at_path")
def test__config__load_user_appdir_config(mock_load_config, mock_path_exists):
"""Test _load_user_appdir_config.
NOTE: We mock `load_config_at_path()` so we can be really focussed with this test
and also not need to actually interact with local home directories.
"""
mock_load_config.side_effect = lambda x: {}
mock_path_exists.side_effect = lambda x: True
_load_user_appdir_config()
# It will check that the default config path exists...
mock_path_exists.assert_has_calls([call(os.path.expanduser("~/.config/sqlfluff"))])
# ...and assuming it does, it will try and load config files at that path.
mock_load_config.assert_has_calls([call(os.path.expanduser("~/.config/sqlfluff"))])
def test__config__toml_list_config():
"""Test Parsing TOML list of values."""
loaded_config = load_config_file(

2
test/fixtures/config/inheritance_a/extra/this_can_have_any_name.cfg Normal file
View File

@@ -0,0 +1,2 @@
[sqlfluff:bar]
foo=foobarextra