Skip to content

Commit 075fece

Browse files
committed
Fix bugs and address feedbacks
1 parent 9196801 commit 075fece

File tree

6 files changed

+126
-29
lines changed

6 files changed

+126
-29
lines changed

src/snowflake/cli/_plugins/remote/commands.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,13 @@ def _display_service_result(
134134
f"✓ Remote Development Environment {service_name} is already running."
135135
)
136136

137-
cc.message(f"VS Code Server URL: {url}")
137+
if url:
138+
cc.message(f"VS Code Server URL: {url}")
139+
else:
140+
cc.warning(
141+
"VS Code Server URL is not yet available. The service is still initializing its public endpoint. "
142+
f"Run 'snow remote start {service_name}' again in a moment to retrieve the URL, or use 'snow remote list' to check status."
143+
)
138144

139145
# Log detailed information at debug level
140146
if stage:
@@ -187,7 +193,7 @@ def _handle_remote_service(
187193
external_access=eai_name,
188194
stage=stage,
189195
image=image,
190-
generate_ssh_key=(ssh_mode is not None and not no_ssh_key),
196+
generate_ssh_key=not no_ssh_key,
191197
)
192198

193199
# Display service result

src/snowflake/cli/_plugins/remote/constants.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ class ServiceResult(enum.Enum):
113113
ENABLE_HEALTH_CHECKS = "false"
114114

115115
# Endpoint readiness validation settings
116-
DEFAULT_ENDPOINT_TIMEOUT_MINUTES = 5 # Maximum time to wait for endpoint readiness
116+
DEFAULT_ENDPOINT_TIMEOUT_MINUTES = 2 # Maximum time to wait for endpoint readiness
117117
ENDPOINT_CHECK_INTERVAL_SECONDS = 5 # Interval between endpoint health checks
118118
ENDPOINT_REQUEST_TIMEOUT_SECONDS = 10 # HTTP request timeout for individual checks
119119

src/snowflake/cli/_plugins/remote/manager.py

Lines changed: 36 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
get_existing_ssh_key,
4040
get_ssh_key_paths,
4141
launch_ide,
42+
sanitize_identifier_component,
4243
setup_ssh_config_with_token,
4344
validate_endpoint_ready,
4445
validate_service_name,
@@ -180,8 +181,9 @@ def _resolve_service_name(self, name_input: str) -> str:
180181

181182
# Otherwise, treat as customer input name and add prefix with Snowflake username
182183
snowflake_username = self._get_current_snowflake_user()
184+
safe_user = sanitize_identifier_component(snowflake_username)
183185
# Convert to uppercase to match Snowflake service naming convention
184-
return f"{SERVICE_NAME_PREFIX}_{snowflake_username}_{name_input}".upper()
186+
return f"{SERVICE_NAME_PREFIX}_{safe_user}_{name_input}".upper()
185187

186188
def _warn_about_config_changes(
187189
self,
@@ -294,17 +296,13 @@ def _handle_existing_service(
294296
ServiceStatus.DELETING.value,
295297
ServiceStatus.DELETED.value,
296298
]:
297-
log.debug(
298-
"Service %s is in failed/error/deleting state (%s), will recreate...",
299-
service_name,
300-
current_status,
299+
cc.warning(
300+
f"Service {service_name} is in failed/error/deleting state ({current_status}), will recreate..."
301301
)
302302
return None
303303
else:
304-
log.debug(
305-
"Service %s has unknown status '%s', will recreate...",
306-
service_name,
307-
current_status,
304+
cc.warning(
305+
f"Service {service_name} has unknown status '{current_status}', will recreate..."
308306
)
309307
return None
310308

@@ -565,40 +563,54 @@ def get_server_ui_url(self, service_name: str) -> str:
565563

566564
def get_validated_server_ui_url(
567565
self, service_name: str, timeout_minutes: int = DEFAULT_ENDPOINT_TIMEOUT_MINUTES
568-
) -> str:
566+
) -> Optional[str]:
569567
"""
570-
Get server UI URL and validate that the endpoint is ready and responding.
568+
Get server UI URL and best-effort validate that the endpoint is ready.
571569
572-
This method combines URL retrieval and endpoint validation to ensure the returned URL
573-
is immediately usable by the user.
570+
This method retrieves the endpoint URL and attempts to validate readiness.
571+
If validation cannot be performed or the endpoint is not yet ready, a warning is
572+
displayed but users can proceed without being blocked.
574573
575574
Args:
576575
service_name: Full name of the service
577576
timeout_minutes: Maximum time to wait for endpoint readiness
578577
579578
Returns:
580-
Validated server UI URL
581-
582-
Raises:
583-
CliError: If URL cannot be retrieved or endpoint doesn't become ready
579+
Server UI URL (returned even if validation fails)
584580
"""
585581
# Get the server UI URL
586582
url = self.get_server_ui_url(service_name)
587583

584+
# Best-effort validation: warn and continue on any issue
588585
try:
589586
self.snowpark_session.sql(
590587
"ALTER SESSION SET python_connector_query_result_format = 'JSON'"
591588
).collect()
592589
token = self.snowpark_session.connection.rest.token
593-
except Exception as e:
594-
raise CliError(
595-
f"Failed to get authentication token for endpoint validation: {e}"
596-
)
597590

598-
if not token:
599-
raise CliError("No authentication token available for endpoint validation")
591+
if not token:
592+
cc.warning(
593+
"Could not obtain authentication token to validate endpoint readiness. "
594+
"Proceeding without validation."
595+
)
596+
else:
597+
try:
598+
validate_endpoint_ready(
599+
url, token, SERVER_UI_ENDPOINT_NAME, timeout_minutes
600+
)
601+
except Exception as e:
602+
# Do not block user; just warn that the endpoint may still be initializing
603+
cc.warning(
604+
f"Endpoint '{SERVER_UI_ENDPOINT_NAME}' may still be initializing. ({e}). Skipping validation."
605+
)
606+
return None
600607

601-
validate_endpoint_ready(url, token, SERVER_UI_ENDPOINT_NAME, timeout_minutes)
608+
except Exception as e:
609+
# Token/session issues: warn and proceed
610+
cc.warning(
611+
f"Skipping endpoint readiness check due to session/token issue: {e}. "
612+
"Proceeding without validation."
613+
)
602614

603615
return url
604616

src/snowflake/cli/_plugins/remote/utils.py

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,39 @@ def validate_service_name(service_name: str) -> None:
9797
)
9898

9999

100+
def sanitize_identifier_component(
101+
value: str, default_value: str = "DEFAULT_USER"
102+
) -> str:
103+
"""
104+
Sanitize a string to be a safe component of a Snowflake identifier.
105+
106+
- Replace invalid characters with underscores
107+
- Collapse multiple underscores
108+
- Ensure it starts with a letter or underscore (prefix underscore if needed)
109+
- Fall back to 'USER' if the result is empty
110+
111+
Args:
112+
value: Input string (e.g., username) to sanitize
113+
114+
Returns:
115+
Sanitized identifier-safe component string
116+
"""
117+
if not value:
118+
return default_value
119+
120+
# Replace any character not allowed in identifiers with underscore
121+
sanitized = re.sub(r"[^A-Za-z0-9_]", "_", value)
122+
123+
# Collapse multiple consecutive underscores
124+
sanitized = re.sub(r"_+", "_", sanitized)
125+
126+
# If everything was stripped to empty or underscores only, fallback
127+
if not sanitized or sanitized.strip("_") == "":
128+
return default_value
129+
130+
return sanitized
131+
132+
100133
class SnowflakeRegion(TypedDict):
101134
region_group: Optional[str]
102135
snowflake_region: str
@@ -667,7 +700,7 @@ def validate_endpoint_ready(
667700
Raises:
668701
CliError: If endpoint doesn't become ready within timeout or authentication fails
669702
"""
670-
log.debug("Validating endpoint readiness for %s at %s", endpoint_name, endpoint_url)
703+
cc.step(f"Validating endpoint readiness for {endpoint_name} at {endpoint_url}")
671704

672705
timeout_seconds = timeout_minutes * 60
673706
start_time = time.time()
@@ -708,6 +741,13 @@ def validate_endpoint_ready(
708741
response.status_code,
709742
ENDPOINT_CHECK_INTERVAL_SECONDS,
710743
)
744+
elapsed = int(time.time() - start_time)
745+
log.debug(
746+
"Still waiting for '%s'... elapsed %ds (last status: %d).",
747+
endpoint_name,
748+
elapsed,
749+
response.status_code,
750+
)
711751

712752
except Exception as e:
713753
log.debug(
@@ -716,6 +756,13 @@ def validate_endpoint_ready(
716756
attempts,
717757
str(e),
718758
)
759+
elapsed = int(time.time() - start_time)
760+
log.debug(
761+
"Still waiting for '%s'... elapsed %ds (last error: %s).",
762+
endpoint_name,
763+
elapsed,
764+
str(e),
765+
)
719766

720767
# Wait before next attempt
721768
time.sleep(ENDPOINT_CHECK_INTERVAL_SECONDS)

tests/remote/test_commands.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ def test_start_command_success(self, mock_start, runner):
6262
external_access=None,
6363
stage=None,
6464
image=None,
65-
generate_ssh_key=False,
65+
generate_ssh_key=True,
6666
)
6767

6868
@patch.object(RemoteManager, "start")

tests/remote/test_manager.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,38 @@ def test_resolve_service_name_with_customer_name(self):
6464
result = manager._resolve_service_name("myproject") # noqa: SLF001
6565
assert result == f"{SERVICE_NAME_PREFIX}_SNOWFLAKE_USER_MYPROJECT"
6666

67+
def test_resolve_service_name_sanitizes_username_special_chars(self):
68+
"""Username with invalid chars should be sanitized in service name."""
69+
manager = RemoteManager()
70+
71+
with patch.object(
72+
manager, "_get_current_snowflake_user", return_value="john.doe+eng@acme-co"
73+
):
74+
result = manager._resolve_service_name("proj1") # noqa: SLF001
75+
76+
# Expected: non-alphanumeric replaced with underscores and collapsed
77+
# "john.doe+eng@acme-co" -> "john_doe_eng_acme_co" then full name uppercased
78+
assert result == f"{SERVICE_NAME_PREFIX}_JOHN_DOE_ENG_ACME_CO_PROJ1"
79+
80+
def test_resolve_service_name_username_empty_uses_default(self):
81+
"""Empty username falls back to DEFAULT_USER in service name."""
82+
manager = RemoteManager()
83+
84+
with patch.object(manager, "_get_current_snowflake_user", return_value=""):
85+
result = manager._resolve_service_name("proj") # noqa: SLF001
86+
87+
assert result == f"{SERVICE_NAME_PREFIX}_DEFAULT_USER_PROJ"
88+
89+
def test_resolve_service_name_keeps_project_name_unchanged_except_case(self):
90+
"""Project name is not sanitized, only uppercased in final name."""
91+
manager = RemoteManager()
92+
93+
with patch.object(manager, "_get_current_snowflake_user", return_value="alice"):
94+
# name contains dashes and dots; per current behavior, it is used as-is and then uppercased
95+
result = manager._resolve_service_name("my-project.v1") # noqa: SLF001
96+
97+
assert result == f"{SERVICE_NAME_PREFIX}_ALICE_MY-PROJECT.V1"
98+
6799
def test_resolve_service_name_with_full_service_name(self):
68100
"""Test that full service names are passed through unchanged."""
69101
manager = RemoteManager()

0 commit comments

Comments
 (0)