Skip to content

Commit 72b39d0

Browse files
committed
SNOW-2306184: ms fixes
1 parent 01e961c commit 72b39d0

File tree

4 files changed

+70
-21
lines changed

4 files changed

+70
-21
lines changed

src/snowflake/cli/_app/loggers.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import logging
1818
import logging.config
1919
from dataclasses import asdict, dataclass, field
20+
from pathlib import Path
2021
from typing import Any, Dict, List
2122

2223
import typer
@@ -136,7 +137,11 @@ def _check_log_level(self, config):
136137

137138
@property
138139
def filename(self):
139-
return self.path.path / _DEFAULT_LOG_FILENAME
140+
from snowflake.cli.api.utils.path_utils import path_resolver
141+
142+
# Ensure Windows short paths are resolved to prevent cleanup issues
143+
resolved_path = path_resolver(str(self.path.path))
144+
return Path(resolved_path) / _DEFAULT_LOG_FILENAME
140145

141146

142147
def create_initial_loggers():

src/snowflake/cli/api/config.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,9 +196,15 @@ def remove_connection_from_proper_file(name: str):
196196

197197
def _get_default_logs_config() -> dict:
198198
"""Get default logs configuration with lazy evaluation to avoid circular imports."""
199+
from snowflake.cli.api.utils.path_utils import path_resolver
200+
201+
config_parent_path = get_config_manager().file_path.parent
202+
# Resolve Windows short paths to prevent issues with temp directory cleanup
203+
resolved_parent_path = path_resolver(str(config_parent_path))
204+
199205
return {
200206
"save_logs": True,
201-
"path": str(get_config_manager().file_path.parent / "logs"),
207+
"path": str(Path(resolved_parent_path) / "logs"),
202208
"level": "info",
203209
}
204210

tests/conftest.py

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -519,11 +519,20 @@ def func():
519519
@contextmanager
520520
def _named_temporary_file(suffix=None, prefix=None):
521521
with tempfile.TemporaryDirectory() as tmp_dir:
522+
# Resolve Windows short paths to prevent cleanup issues
523+
from snowflake.cli.api.utils.path_utils import path_resolver
524+
525+
resolved_tmp_dir = path_resolver(tmp_dir)
526+
522527
suffix = suffix or ""
523528
prefix = prefix or ""
524-
f = Path(tmp_dir) / f"{prefix}tmp_file{suffix}"
529+
f = Path(resolved_tmp_dir) / f"{prefix}tmp_file{suffix}"
525530
f.touch()
526-
yield f
531+
try:
532+
yield f
533+
finally:
534+
# Ensure all logging handlers are closed before temp directory cleanup
535+
clean_logging_handlers()
527536

528537

529538
@pytest.fixture()

tests/test_config.py

Lines changed: 46 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,23 @@
3535

3636

3737
def test_empty_config_file_is_created_if_not_present():
38+
from snowflake.cli.api.utils.path_utils import path_resolver
39+
40+
from tests.conftest import clean_logging_handlers
41+
3842
with TemporaryDirectory() as tmp_dir:
39-
config_file = Path(tmp_dir) / "sub" / "config.toml"
43+
# Resolve Windows short paths to prevent cleanup issues
44+
resolved_tmp_dir = path_resolver(tmp_dir)
45+
config_file = Path(resolved_tmp_dir) / "sub" / "config.toml"
4046
assert config_file.exists() is False
4147

42-
config_init(config_file)
43-
assert config_file.exists() is True
44-
45-
assert_file_permissions_are_strict(config_file)
48+
try:
49+
config_init(config_file)
50+
assert config_file.exists() is True
51+
assert_file_permissions_are_strict(config_file)
52+
finally:
53+
# Ensure all logging handlers are closed before temp directory cleanup
54+
clean_logging_handlers()
4655

4756

4857
@mock.patch.dict(os.environ, {}, clear=True)
@@ -161,16 +170,26 @@ def test_get_all_connections(test_snowcli_config):
161170
def test_create_default_config_if_not_exists_with_proper_permissions(
162171
mock_get_config_section,
163172
):
173+
from snowflake.cli.api.utils.path_utils import path_resolver
174+
175+
from tests.conftest import clean_logging_handlers
176+
164177
mock_get_config_section.return_value = {}
165178
with TemporaryDirectory() as tmp_dir:
166-
config_path = Path(f"{tmp_dir}/snowflake/config.toml")
179+
# Resolve Windows short paths to prevent cleanup issues
180+
resolved_tmp_dir = path_resolver(tmp_dir)
181+
config_path = Path(f"{resolved_tmp_dir}/snowflake/config.toml")
167182

168-
# Test the config initialization with a specific path
169-
config_init(config_path)
183+
try:
184+
# Test the config initialization with a specific path
185+
config_init(config_path)
170186

171-
assert config_path.exists()
172-
assert_file_permissions_are_strict(config_path.parent)
173-
assert_file_permissions_are_strict(config_path)
187+
assert config_path.exists()
188+
assert_file_permissions_are_strict(config_path.parent)
189+
assert_file_permissions_are_strict(config_path)
190+
finally:
191+
# Ensure all logging handlers are closed before temp directory cleanup
192+
clean_logging_handlers()
174193

175194

176195
@mock.patch.dict(
@@ -496,8 +515,14 @@ def test_too_wide_permissions_on_custom_config_file_causes_warning(
496515
def test_too_wide_permissions_on_custom_config_file_causes_warning_windows(permissions):
497516
import subprocess
498517

518+
from snowflake.cli.api.utils.path_utils import path_resolver
519+
520+
from tests.conftest import clean_logging_handlers
521+
499522
with TemporaryDirectory() as tmp_dir:
500-
config_path = Path(tmp_dir) / "config.toml"
523+
# Resolve Windows short paths to prevent cleanup issues
524+
resolved_tmp_dir = path_resolver(tmp_dir)
525+
config_path = Path(resolved_tmp_dir) / "config.toml"
501526
config_path.touch()
502527
result = subprocess.run(
503528
["icacls", str(config_path), "/GRANT", f"Everyone:{permissions}"],
@@ -506,11 +531,15 @@ def test_too_wide_permissions_on_custom_config_file_causes_warning_windows(permi
506531
)
507532
assert result.returncode == 0, result.stdout + result.stderr
508533

509-
with pytest.warns(
510-
UserWarning,
511-
match=r"Unauthorized users \(.*\) have access to configuration file .*",
512-
):
513-
config_init(config_file=config_path)
534+
try:
535+
with pytest.warns(
536+
UserWarning,
537+
match=r"Unauthorized users \(.*\) have access to configuration file .*",
538+
):
539+
config_init(config_file=config_path)
540+
finally:
541+
# Ensure all logging handlers are closed before temp directory cleanup
542+
clean_logging_handlers()
514543

515544

516545
@parametrize_chmod

0 commit comments

Comments
 (0)