Skip to content

Commit 097f9be

Browse files
sfc-gh-anavalosSnowflake Authors
andauthored
Project import generated by Copybara. (#170)
GitOrigin-RevId: 964e61b5d23d49642fd754c11e3f0b2707e79ccd Co-authored-by: Snowflake Authors <[email protected]>
1 parent 2bd6eac commit 097f9be

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

52 files changed

+2353
-1303
lines changed

.pre-commit-config.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ repos:
66
- id: check-py-test-feature-tags
77
name: Check py_test feature area tags
88
description: Ensure all py_test targets have feature area tags
9-
entry: python bazel/check_feature_tags.py --precommit
9+
entry: python3 bazel/check_feature_tags.py --precommit
1010
language: system
1111
files: BUILD\.bazel$
1212
stages:

CHANGELOG.md

Lines changed: 504 additions & 491 deletions
Large diffs are not rendered by default.

ci/RunBazelAction.sh

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#!/bin/bash
22
# DESCRIPTION: Utility Shell script to run bazel action for snowml repository
33
#
4-
# RunBazelAction.sh <test|coverage> [-b <bazel_path>] [-m merge_gate|continuous_run|quarantined|local_unittest|local_all] [-t <target>] [-c <path_to_coverage_report>]
4+
# RunBazelAction.sh <test|coverage> [-b <bazel_path>] [-m merge_gate|continuous_run|quarantined|local_unittest|local_all] [-t <target>] [-c <path_to_coverage_report>] [--tags <tags>]
55
#
66
# Args:
77
# action: bazel action, choose from test and coverage
@@ -17,6 +17,7 @@
1717
# -t: specify the target for local_unit and local_all mode
1818
# -c: specify the path to the coverage report dat file.
1919
# -e: specify the environment, used to determine.
20+
# --tags: specify bazel test tag filters (e.g., "feature:jobs,feature:data")
2021
#
2122

2223
set -o pipefail
@@ -28,13 +29,21 @@ mode="continuous_run"
2829
target=""
2930
SF_ENV="prod3"
3031
WITH_SPCS_IMAGE=false
32+
TAG_FILTERS=""
3133
PROG=$0
3234

3335
action=$1 && shift
3436

3537
help() {
3638
local exit_code=$1
37-
echo "Usage: ${PROG} <test|coverage> [-b <bazel_path>] [-m merge_gate|continuous_run|quarantined|local_unittest|local_all|perf] [-e <snowflake_env>] [--with-spcs-image]"
39+
echo "Usage: ${PROG} <test|coverage> [-b <bazel_path>] [-m merge_gate|continuous_run|quarantined|local_unittest|local_all|perf] [-e <snowflake_env>] [--tags <tags>] [--with-spcs-image]"
40+
echo ""
41+
echo "Options:"
42+
echo " --tags <tags> Specify bazel tag filters (comma-separated)"
43+
echo ""
44+
echo "Examples:"
45+
echo " ${PROG} test --tags 'feature:jobs'"
46+
echo " ${PROG} test --tags 'feature:jobs,feature:data'"
3847
exit "${exit_code}"
3948
}
4049

@@ -72,6 +81,10 @@ while (($#)); do
7281
shift
7382
SF_ENV=$1
7483
;;
84+
--tags)
85+
shift
86+
TAG_FILTERS="$1"
87+
;;
7588
--with-spcs-image)
7689
WITH_SPCS_IMAGE=true
7790
;;
@@ -98,13 +111,20 @@ action_env=()
98111
if [[ "${WITH_SPCS_IMAGE}" = true ]]; then
99112
export SKIP_GRYPE=true
100113
source model_container_services_deployment/ci/build_and_push_images.sh
101-
action_env=("--action_env=BUILDER_IMAGE_PATH=${BUILDER_IMAGE_PATH}" "--action_env=BASE_CPU_IMAGE_PATH=${BASE_CPU_IMAGE_PATH}" "--action_env=BASE_GPU_IMAGE_PATH=${BASE_GPU_IMAGE_PATH}" "--action_env=IMAGE_BUILD_SIDECAR_CPU_PATH=${IMAGE_BUILD_SIDECAR_CPU_PATH}" "--action_env=IMAGE_BUILD_SIDECAR_GPU_PATH=${IMAGE_BUILD_SIDECAR_GPU_PATH}" "--action_env=PROXY_IMAGE_PATH=${PROXY_IMAGE_PATH}")
114+
action_env=("--action_env=BUILDER_IMAGE_PATH=${BUILDER_IMAGE_PATH}" "--action_env=BASE_CPU_IMAGE_PATH=${BASE_CPU_IMAGE_PATH}" "--action_env=BASE_GPU_IMAGE_PATH=${BASE_GPU_IMAGE_PATH}" "--action_env=IMAGE_BUILD_SIDECAR_CPU_PATH=${IMAGE_BUILD_SIDECAR_CPU_PATH}" "--action_env=IMAGE_BUILD_SIDECAR_GPU_PATH=${IMAGE_BUILD_SIDECAR_GPU_PATH}" "--action_env=PROXY_IMAGE_PATH=${PROXY_IMAGE_PATH}" "--action_env=VLLM_IMAGE_PATH=${VLLM_IMAGE_PATH}")
102115
fi
103116

104117
working_dir=$(mktemp -d "/tmp/tmp_XXXXX")
105118
trap 'rm -rf "${working_dir}"' EXIT
106119

107-
tag_filter="--test_tag_filters="
120+
# Set up tag filtering
121+
if [[ -n "${TAG_FILTERS:-}" ]]; then
122+
tag_filter="--test_tag_filters=${TAG_FILTERS}"
123+
echo "Running with tag filters: ${TAG_FILTERS}"
124+
else
125+
tag_filter="--test_tag_filters="
126+
fi
127+
108128
cache_test_results="--cache_test_results=no"
109129

110130
case "${mode}" in

ci/conda_recipe/meta.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ build:
1717
noarch: python
1818
package:
1919
name: snowflake-ml-python
20-
version: 1.9.2
20+
version: 1.10.0
2121
requirements:
2222
build:
2323
- python

snowflake/ml/_internal/utils/service_logger.py

Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,15 @@
99

1010
import platformdirs
1111

12+
# Module-level logger for operational messages that should appear on console
13+
stdout_handler = logging.StreamHandler(sys.stdout)
14+
stdout_handler.setFormatter(logging.Formatter("%(message)s"))
15+
16+
console_logger = logging.getLogger(__name__)
17+
console_logger.addHandler(stdout_handler)
18+
console_logger.setLevel(logging.INFO)
19+
console_logger.propagate = False
20+
1221

1322
class LogColor(enum.Enum):
1423
GREY = "\x1b[38;20m"
@@ -109,49 +118,54 @@ def _get_or_create_parent_logger(operation_id: str) -> logging.Logger:
109118
"""Get or create a parent logger with FileHandler for the operation."""
110119
parent_logger_name = f"snowflake_ml_operation_{operation_id}"
111120
parent_logger = logging.getLogger(parent_logger_name)
121+
parent_logger.setLevel(logging.DEBUG)
122+
parent_logger.propagate = False
112123

113-
# Only add handler if it doesn't exist yet
114124
if not parent_logger.handlers:
115125
log_file_path = _get_log_file_path(operation_id)
116126

117127
if log_file_path:
118-
# Successfully found a writable location
119128
try:
120129
file_handler = logging.FileHandler(log_file_path)
121130
file_handler.setFormatter(logging.Formatter("%(name)s [%(asctime)s] [%(levelname)s] %(message)s"))
122131
parent_logger.addHandler(file_handler)
123-
parent_logger.setLevel(logging.DEBUG)
124-
parent_logger.propagate = False # Don't propagate to root logger
125132

126-
# Log the file location
127-
parent_logger.warning(f"Operation logs saved to: {log_file_path}")
133+
console_logger.info(f"create_service logs saved to: {log_file_path}")
128134
except OSError as e:
129-
# Even though we found a path, file creation failed
130-
# Fall back to console-only logging
131-
parent_logger.setLevel(logging.DEBUG)
132-
parent_logger.propagate = False
133-
parent_logger.warning(f"Could not create log file at {log_file_path}: {e}. Using console-only logging.")
135+
console_logger.warning(f"Could not create log file at {log_file_path}: {e}.")
134136
else:
135137
# No writable location found, use console-only logging
136-
parent_logger.setLevel(logging.DEBUG)
137-
parent_logger.propagate = False
138-
parent_logger.warning("Filesystem appears to be readonly. Using console-only logging.")
138+
console_logger.warning("No writable location found for create_service log file.")
139+
140+
if logging.getLogger().level > logging.INFO:
141+
console_logger.info(
142+
"To see logs in console, set log level to INFO: logging.getLogger().setLevel(logging.INFO)"
143+
)
139144

140145
return parent_logger
141146

142147

143148
def get_logger(logger_name: str, info_color: LogColor, operation_id: Optional[str] = None) -> logging.Logger:
144149
logger = logging.getLogger(logger_name)
145-
handler = logging.StreamHandler(sys.stdout)
146-
handler.setFormatter(CustomFormatter(info_color))
147-
logger.addHandler(handler)
150+
root_logger = logging.getLogger()
148151

149152
# If operation_id provided, set up parent logger with file handler
150153
if operation_id:
151154
parent_logger = _get_or_create_parent_logger(operation_id)
152155
logger.parent = parent_logger
153156
logger.propagate = True
154157

158+
if root_logger.level <= logging.INFO:
159+
handler = logging.StreamHandler(sys.stdout)
160+
handler.setFormatter(CustomFormatter(info_color))
161+
logger.addHandler(handler)
162+
else:
163+
# No operation_id - add console handler only if user wants verbose logging
164+
if root_logger.level <= logging.INFO and not logger.handlers:
165+
handler = logging.StreamHandler(sys.stdout)
166+
handler.setFormatter(CustomFormatter(info_color))
167+
logger.addHandler(handler)
168+
155169
return logger
156170

157171

snowflake/ml/_internal/utils/service_logger_test.py

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,17 @@ def setUp(self) -> None:
1717
self.test_logger_name = "test_logger"
1818
self.test_color = service_logger.LogColor.BLUE
1919

20+
# Store original root logger level
21+
self._original_root_level = logging.getLogger().level
22+
2023
# Clear any existing loggers to avoid test interference
2124
self._clear_all_loggers()
2225

2326
def tearDown(self) -> None:
2427
"""Clean up test fixtures."""
28+
# Restore original root logger level
29+
logging.getLogger().setLevel(self._original_root_level)
30+
2531
# Clear loggers created during tests
2632
self._clear_all_loggers()
2733

@@ -45,9 +51,12 @@ def _clear_all_loggers(self) -> None:
4551

4652
def test_get_logger_without_operation_id(self) -> None:
4753
"""Test that get_logger works without operation_id (backward compatibility)."""
54+
# Set root logger to INFO level to ensure handler is added
55+
logging.getLogger().setLevel(logging.INFO)
56+
4857
logger = service_logger.get_logger(self.test_logger_name, self.test_color)
4958

50-
# Should have one handler (StreamHandler)
59+
# Should have one handler (StreamHandler) when in verbose mode
5160
self.assertEqual(len(logger.handlers), 1)
5261
self.assertIsInstance(logger.handlers[0], logging.StreamHandler)
5362

@@ -58,11 +67,24 @@ def test_get_logger_without_operation_id(self) -> None:
5867
# Should use custom formatter
5968
self.assertIsInstance(logger.handlers[0].formatter, service_logger.CustomFormatter)
6069

70+
def test_get_logger_without_operation_id_quiet_mode(self) -> None:
71+
"""Test that get_logger without operation_id doesn't add handler in quiet mode."""
72+
# Set root logger to WARNING level (quiet mode)
73+
logging.getLogger().setLevel(logging.WARNING)
74+
75+
logger = service_logger.get_logger(self.test_logger_name, self.test_color)
76+
77+
# Should have no handlers in quiet mode
78+
self.assertEqual(len(logger.handlers), 0)
79+
6180
def test_get_logger_with_operation_id(self) -> None:
62-
"""Test that get_logger creates parent logger with operation_id."""
81+
"""Test that get_logger creates parent logger with operation_id in verbose mode."""
82+
# Set root logger to INFO level to ensure handler is added
83+
logging.getLogger().setLevel(logging.INFO)
84+
6385
logger = service_logger.get_logger(self.test_logger_name, self.test_color, self.test_operation_id)
6486

65-
# Should have one handler (StreamHandler)
87+
# Should have one handler (StreamHandler) in verbose mode
6688
self.assertEqual(len(logger.handlers), 1)
6789
self.assertIsInstance(logger.handlers[0], logging.StreamHandler)
6890

@@ -74,6 +96,24 @@ def test_get_logger_with_operation_id(self) -> None:
7496
# Should propagate to parent
7597
self.assertTrue(logger.propagate)
7698

99+
def test_get_logger_with_operation_id_quiet_mode(self) -> None:
100+
"""Test that get_logger with operation_id doesn't add console handler in quiet mode."""
101+
# Set root logger to WARNING level (quiet mode)
102+
logging.getLogger().setLevel(logging.WARNING)
103+
104+
logger = service_logger.get_logger(self.test_logger_name, self.test_color, self.test_operation_id)
105+
106+
# Should have no console handlers in quiet mode (only file via propagation)
107+
self.assertEqual(len(logger.handlers), 0)
108+
109+
# Should still have parent logger for file output
110+
self.assertIsNotNone(logger.parent)
111+
if logger.parent:
112+
self.assertEqual(logger.parent.name, f"snowflake_ml_operation_{self.test_operation_id}")
113+
114+
# Should still propagate to parent for file logging
115+
self.assertTrue(logger.propagate)
116+
77117
def test_parent_logger_creation_with_file(self) -> None:
78118
"""Test that parent logger is created with FileHandler when filesystem is writable."""
79119
with mock.patch("snowflake.ml._internal.utils.service_logger._get_log_file_path") as mock_path:

snowflake/ml/experiment/BUILD.bazel

Lines changed: 17 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2,29 +2,6 @@ load("//bazel:py_rules.bzl", "py_library", "py_package", "py_test")
22

33
package(default_visibility = ["//visibility:public"])
44

5-
py_library(
6-
name = "callback",
7-
srcs = ["callback.py"],
8-
deps = [
9-
"//snowflake/ml/experiment:experiment_tracking",
10-
"//snowflake/ml/model:model_signature",
11-
],
12-
)
13-
14-
py_test(
15-
name = "callback_test",
16-
srcs = ["callback_test.py"],
17-
optional_dependencies = [
18-
"lightgbm",
19-
],
20-
tags = ["feature:observability"],
21-
deps = [
22-
":callback",
23-
"//snowflake/ml/experiment:experiment_tracking",
24-
"//snowflake/ml/model:model_signature",
25-
],
26-
)
27-
285
py_library(
296
name = "experiment_info",
307
srcs = ["_experiment_info.py"],
@@ -38,7 +15,10 @@ py_test(
3815
srcs = ["_experiment_info_test.py"],
3916
main = "_experiment_info_test.py",
4017
tags = ["feature:observability"],
41-
deps = [":experiment_info"],
18+
deps = [
19+
":experiment_info",
20+
"//snowflake/ml/test_utils:mock_progress",
21+
],
4222
)
4323

4424
py_library(
@@ -62,12 +42,24 @@ py_test(
6242
],
6343
)
6444

45+
py_library(
46+
name = "utils",
47+
srcs = ["utils.py"],
48+
)
49+
50+
py_test(
51+
name = "utils_test",
52+
srcs = ["utils_test.py"],
53+
tags = ["feature:observability"],
54+
deps = [":utils"],
55+
)
56+
6557
py_library(
6658
name = "experiment",
6759
srcs = ["__init__.py"],
6860
deps = [
69-
":callback",
7061
":experiment_tracking",
62+
"//snowflake/ml/experiment/callback",
7163
],
7264
)
7365

snowflake/ml/experiment/_experiment_info_test.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
from snowflake.ml.experiment import _experiment_info as experiment_info
66
from snowflake.ml.registry._manager.model_manager import ModelManager
7+
from snowflake.ml.test_utils.mock_progress import create_mock_progress_status
78

89

910
class ExperimentInfoPatcherTest(absltest.TestCase):
@@ -89,6 +90,7 @@ def test_preserves_original_method_signature_and_return_value(self) -> None:
8990
experiment_info.ExperimentInfoPatcher._original_log_model = mock_original
9091

9192
mock_manager = mock.MagicMock(spec=ModelManager)
93+
mock_progress_status = create_mock_progress_status()
9294

9395
with experiment_info.ExperimentInfoPatcher(self.experiment_info):
9496
# Call with various arguments
@@ -99,6 +101,7 @@ def test_preserves_original_method_signature_and_return_value(self) -> None:
99101
version_name="v1.0",
100102
comment="test comment",
101103
metrics={"accuracy": 0.95},
104+
progress_status=mock_progress_status,
102105
)
103106

104107
# Verify all arguments are passed through with experiment_info added
@@ -109,6 +112,7 @@ def test_preserves_original_method_signature_and_return_value(self) -> None:
109112
version_name="v1.0",
110113
comment="test comment",
111114
metrics={"accuracy": 0.95},
115+
progress_status=mock_progress_status,
112116
experiment_info=self.experiment_info,
113117
)
114118

0 commit comments

Comments
 (0)