From b1e7d4d637cc5326d7cd1cb74e71e8e1aed21588 Mon Sep 17 00:00:00 2001 From: Mikita Sakalouski Date: Mon, 2 Dec 2024 19:42:49 +0100 Subject: [PATCH 1/3] SNOW-1825673 use tmdir in case of Permission error --- src/snowflake/connector/sf_dirs.py | 40 ++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/src/snowflake/connector/sf_dirs.py b/src/snowflake/connector/sf_dirs.py index 09164affb..3c75ed0c1 100644 --- a/src/snowflake/connector/sf_dirs.py +++ b/src/snowflake/connector/sf_dirs.py @@ -7,7 +7,9 @@ import os import pathlib from functools import cached_property +from tempfile import TemporaryDirectory from typing import Protocol +from warnings import warn from platformdirs import PlatformDirs @@ -34,17 +36,33 @@ def _resolve_platform_dirs() -> PlatformDirsProto: snowflake_home = pathlib.Path( os.environ.get("SNOWFLAKE_HOME", "~/.snowflake/"), ).expanduser() - if snowflake_home.exists(): - return SFPlatformDirs( - str(snowflake_home), - **platformdir_kwargs, - ) - else: - # In case SNOWFLAKE_HOME does not exist we fall back to using - # platformdirs to determine where system files should be placed. Please - # see docs for all the directories defined in the module at - # https://platformdirs.readthedocs.io/ - return PlatformDirs(**platformdir_kwargs) + try: + if snowflake_home.exists(): + return SFPlatformDirs( + str(snowflake_home), + **platformdir_kwargs, + ) + else: + # In case SNOWFLAKE_HOME does not exist we fall back to using + # platformdirs to determine where system files should be placed. Please + # see docs for all the directories defined in the module at + # https://platformdirs.readthedocs.io/ + return PlatformDirs(**platformdir_kwargs) + except PermissionError as pe: + if os.environ.get("SNOWFLAKE_HOME") is None: + tmp_dir = TemporaryDirectory( + suffix="_snowflake", ignore_cleanup_errors=True + ) + warn( + f"Received permission error while checking if SNOWFLAKE_HOME exists. Continue with temporary direcoty `{tmp_dir}`.\n" + f"Original Error: {pe}" + ) + return SFPlatformDirs( + str(tmp_dir), + **platformdir_kwargs, + ) + else: + raise pe class SFPlatformDirs: From 7b309552cc1906cdbfc64ead4c0c675cd4d4a8e5 Mon Sep 17 00:00:00 2001 From: Mikita Sakalouski Date: Mon, 2 Dec 2024 20:14:04 +0100 Subject: [PATCH 2/3] fix: correct temporary directory name in permission error handling and add unit test --- src/snowflake/connector/sf_dirs.py | 4 ++-- test/unit/test_connector_sf_dirs.py | 11 +++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) create mode 100644 test/unit/test_connector_sf_dirs.py diff --git a/src/snowflake/connector/sf_dirs.py b/src/snowflake/connector/sf_dirs.py index 3c75ed0c1..673e8fcf4 100644 --- a/src/snowflake/connector/sf_dirs.py +++ b/src/snowflake/connector/sf_dirs.py @@ -54,11 +54,11 @@ def _resolve_platform_dirs() -> PlatformDirsProto: suffix="_snowflake", ignore_cleanup_errors=True ) warn( - f"Received permission error while checking if SNOWFLAKE_HOME exists. Continue with temporary direcoty `{tmp_dir}`.\n" + f"Received permission error while checking if SNOWFLAKE_HOME exists. Continue with temporary direcoty `{tmp_dir.name}`.\n" f"Original Error: {pe}" ) return SFPlatformDirs( - str(tmp_dir), + str(tmp_dir.name), **platformdir_kwargs, ) else: diff --git a/test/unit/test_connector_sf_dirs.py b/test/unit/test_connector_sf_dirs.py new file mode 100644 index 000000000..2bf7ff6d6 --- /dev/null +++ b/test/unit/test_connector_sf_dirs.py @@ -0,0 +1,11 @@ +from unittest import mock + +from snowflake.connector.sf_dirs import SFPlatformDirs, _resolve_platform_dirs + + +@mock.patch("pathlib.Path.exists", side_effect=PermissionError) +def test_snowflake_home_permission_error(self): + platform_dirs = _resolve_platform_dirs() + assert isinstance(platform_dirs, SFPlatformDirs) + assert platform_dirs.user_config_path.name.endswith("_snowflake") + assert platform_dirs.user_config_path.name.startswith("tmp") From 0beb6c182f4ba9296812aa7a3204aa6e6ee0734e Mon Sep 17 00:00:00 2001 From: Mikita Sakalouski <38785549+mikita-sakalouski@users.noreply.github.com> Date: Mon, 2 Dec 2024 20:30:26 +0100 Subject: [PATCH 3/3] fix: remove if, adjust warning --- src/snowflake/connector/sf_dirs.py | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/src/snowflake/connector/sf_dirs.py b/src/snowflake/connector/sf_dirs.py index 673e8fcf4..8d7f787f5 100644 --- a/src/snowflake/connector/sf_dirs.py +++ b/src/snowflake/connector/sf_dirs.py @@ -49,20 +49,17 @@ def _resolve_platform_dirs() -> PlatformDirsProto: # https://platformdirs.readthedocs.io/ return PlatformDirs(**platformdir_kwargs) except PermissionError as pe: - if os.environ.get("SNOWFLAKE_HOME") is None: - tmp_dir = TemporaryDirectory( - suffix="_snowflake", ignore_cleanup_errors=True - ) - warn( - f"Received permission error while checking if SNOWFLAKE_HOME exists. Continue with temporary direcoty `{tmp_dir.name}`.\n" - f"Original Error: {pe}" - ) - return SFPlatformDirs( - str(tmp_dir.name), - **platformdir_kwargs, - ) - else: - raise pe + tmp_dir = TemporaryDirectory( + suffix="_snowflake", ignore_cleanup_errors=True + ) + warn( + f"Received permission error while checking if {snowflake_home} exists. Continue with temporary direcoty `{tmp_dir.name}`.\n" + f"Original Error: {pe}" + ) + return SFPlatformDirs( + str(tmp_dir.name), + **platformdir_kwargs, + ) class SFPlatformDirs: