Remote Dev Plugin Part 4 - bug fix and feature improvement by sfc-gh-zzhu · Pull Request #2569 · snowflakedb/snowflake-cli

sfc-gh-zzhu · 2025-08-28T22:32:33Z

Pre-review checklist

I've confirmed that instructions included in README.md are still correct after my changes in the codebase.
I've added or updated automated unit tests to verify correctness of my new code.
I've added or updated integration tests to verify correctness of my new code.
I've confirmed that my changes are working by executing CLI's commands manually on MacOS.
I've confirmed that my changes are working by executing CLI's commands manually on Windows.
I've confirmed that my changes are up-to-date with the target branch.
I've described my changes in the release notes.
I've described my changes in the section below.
I've described my changes in the documentation.

Changes description

This PR enhances the snow remote command with several new features and improvements:

IDE Integration:
- Added --code flag to launch VS Code connected to the remote service
- Added --cursor flag to launch Cursor connected to the remote service
Improved Stage Mounting:
- Updated mount paths to ensure VS Code extensions got excluded from persistence storage.
- Improved help text to better explain stage mounting behavior
Error Handling:
- Replaced generic ValueError with more specific CliError for better error messages
- Added validation for service names to prevent CREATE SERVICE failures
- Improved error messages for common failure scenarios
User Experience:
- Added warnings when configuration changes are provided for existing services
- Improved help text with more detailed examples
- Enhanced command output for better user feedback

The changes maintain backward compatibility while adding new functionality for IDE integration and persistent storage.

sfc-gh-zzhu · 2025-08-28T22:32:53Z

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

graphite-app · 2025-08-29T16:38:06Z

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

+
+    folder_uri = f"vscode-remote://ssh-remote+{service_name}{remote_path}"
+    try:
+        subprocess.run([binary_name, "--folder-uri", folder_uri], check=False)


The subprocess.run() call uses check=False, which means IDE launch failures won't raise exceptions. This could create a misleading user experience where the CLI appears to succeed even when the IDE fails to start. Consider either:

Using check=True to automatically raise exceptions on non-zero exit codes, or

Capturing the return value and checking it manually:
result = subprocess.run([binary_name, "--folder-uri", folder_uri], check=False) if result.returncode != 0: raise CliError(f"Failed to launch {binary_name}, exit code: {result.returncode}")

This would provide clearer feedback to users when their IDE fails to launch.

Suggested change

subprocess.run([binary_name, "--folder-uri", folder_uri], check=False)

result = subprocess.run([binary_name, "--folder-uri", folder_uri], check=False)

if result.returncode != 0:

raise CliError(f"Failed to launch {binary_name}, exit code: {result.returncode}")

Spotted by Diamond

Is this helpful? React 👍 or 👎 to let us know.

@sfc-gh-zzhu

sfc-gh-dhung · 2025-08-29T18:38:00Z

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

+        help="Internal Snowflake stage to mount for persistent storage. "
+        "The stage will be mounted at two locations: "
+        "1) 'stage/user-default' -> '/root/user-default' (workspace files), "
+        "2) 'stage/.vscode-server/data' -> '/root/.vscode-server/data' (VS Code settings). "
+        "Example: --stage @my_stage or --stage @my_stage/project",


Just curious what are the considerations for using block storage instead? Is there a reason we're not going with that currently?

sfc-gh-dhung · 2025-08-29T18:39:14Z

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

        "--ssh",
        help="Set up SSH configuration for connecting to the remote environment. This is a blocking command that keeps SSH connections alive.",
    ),
+    code: bool = typer.Option(
+        False,
+        "--code",
+        help="Open VS Code connected to the remote service over SSH (mutually exclusive with --ssh and --cursor)",
+    ),
+    cursor: bool = typer.Option(
+        False,
+        "--cursor",
+        help="Open Cursor connected to the remote service over SSH (mutually exclusive with --ssh and --code)",
+    ),


WDYT of making these full subcommands rather than options? i.e. snow remote start cursor instead of snow remote start --cursor

or alternatively flatten it a bit: snow remote cursor. Basically the commands would be:
snow remote start - start the service in the background, display the URL needed to connect by web interface
snow remote ssh - blocking command*, opens SSH tunnel
snow remote code - blocking command, open SSH tunnel + VSCode. As a bonus would be neat if this automatically exits if the user closes VSCode
snow remote cursor - same idea

*could consider making this non-blocking and spin up a bg process in the future, tbd

sfc-gh-dhung · 2025-08-29T18:42:46Z

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

+        launching_ide = code or cursor
+        if launching_ide:
+            manager.validate_ide_requirements(name, eai_name)


nit: doesn't it make sense to just ssh = ssh or launching_ide since IDEs presumably will require ssh?

sfc-gh-dhung · 2025-08-29T18:44:06Z

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

+            cc.warning(
+                f"Service '{service_name}' is already running. "
+                "Configuration changes provided to 'snow remote start' will be ignored for existing services. "
+                f"To apply changes, delete the service first with 'snow remote delete {service_name}' and then start it again with the new configuration."
+            )


consider making this an error instead

sfc-gh-dhung · 2025-08-29T18:44:43Z

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

+
+            # Check if user provided configuration options that would modify the service
+            self._warn_about_config_changes(service_name, **config_options)
+


nit: consider refactoring so we're not calling this in three places

sfc-gh-dhung · 2025-08-29T18:48:07Z

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

+
+    folder_uri = f"vscode-remote://ssh-remote+{service_name}{remote_path}"
+    try:
+        subprocess.run([binary_name, "--folder-uri", folder_uri], check=False)


@sfc-gh-zzhu

graphite-app · 2025-09-05T00:49:41Z

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

+
                if not token:
                    cc.step("❌ Unable to get token. Retrying in 30 seconds...")
-                    self._wait_with_shutdown_check(SSH_RETRY_INTERVAL)
+                    self._interruptible_sleep(SSH_RETRY_INTERVAL)


There's an inconsistency between error handling approaches. The _get_fresh_token() method now raises exceptions rather than returning None, but this code still checks for None values. Consider updating this exception handling to align with the new behavior:

try: token = self._get_fresh_token() except Exception as e: cc.step(f"❌ Unable to get token: {e}. Retrying in 30 seconds...") self._interruptible_sleep(SSH_RETRY_INTERVAL) continue

This would properly catch and handle exceptions from the updated _get_fresh_token() implementation.

Suggested change

if not token:

cc.step("❌ Unable to get token. Retrying in 30 seconds...")

self._wait_with_shutdown_check(SSH_RETRY_INTERVAL)

self._interruptible_sleep(SSH_RETRY_INTERVAL)

try:

token = self._get_fresh_token()

except Exception as e:

cc.step(f"❌ Unable to get token: {e}. Retrying in 30 seconds...")

self._interruptible_sleep(SSH_RETRY_INTERVAL)

continue

Spotted by Diamond

Is this helpful? React 👍 or 👎 to let us know.

graphite-app · 2025-09-05T20:32:09Z

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

+            response = requests.get(
+                f"https://{endpoint_url}",
+                headers=headers,
+                timeout=ENDPOINT_REQUEST_TIMEOUT_SECONDS,
+            )


Security Concern: URL Construction

The current URL construction in validate_endpoint_ready() may be vulnerable to protocol downgrade attacks. When prepending https:// to endpoint_url without validating its content, there's a risk of creating malformed URLs if endpoint_url already contains a protocol scheme.

For example, if endpoint_url contains http://example.com, the resulting URL would be https://http://example.com, which could lead to unexpected behavior or security issues.

Consider either:

Validating that endpoint_url doesn't already contain a protocol scheme

Using a URL parsing library to properly construct the URL with HTTPS

# Example fix using validation if "://" in endpoint_url: raise CliError(f"Endpoint URL should not contain protocol: {endpoint_url}") response = requests.get(f"https://{endpoint_url}", ...) # Or using URL parsing from urllib.parse import urlparse, urlunparse parsed = urlparse(endpoint_url) if parsed.scheme: # Strip existing scheme and ensure HTTPS parts = list(parsed) parts[0] = "https" url = urlunparse(parts) else: url = f"https://{endpoint_url}" response = requests.get(url, ...)

Suggested change

response = requests.get(

f"https://{endpoint_url}",

headers=headers,

timeout=ENDPOINT_REQUEST_TIMEOUT_SECONDS,

)

if "://" in endpoint_url:

raise CliError(f"Endpoint URL should not contain protocol: {endpoint_url}")

response = requests.get(

f"https://{endpoint_url}",

headers=headers,

timeout=ENDPOINT_REQUEST_TIMEOUT_SECONDS,

)

Spotted by Diamond

Is this helpful? React 👍 or 👎 to let us know.

graphite-app · 2025-09-05T20:32:10Z

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

            # If result is None, service needs recreation - continue to creation
        else:
-            log.debug("Service %s does not exist, creating...", service_name)
+            cc.step(f"Service {service_name} does not exist, creating...")


Inconsistent logging level for service creation message

There's an inconsistency in how service creation is logged. When a service doesn't exist:

cc.step(f"Service {service_name} does not exist, creating...")

This uses cc.step() which displays to users, while the similar check above uses:

log.debug("Service %s is pending, waiting for it to be ready...", service_name)

For consistency, either:

Use log.debug() for both messages to keep implementation details hidden from users

Use cc.step() for both to provide consistent user-facing progress updates

This would make the logging behavior more predictable and maintain a consistent level of verbosity in the user interface.

Spotted by Diamond

Is this helpful? React 👍 or 👎 to let us know.

This was referenced Aug 28, 2025

Remote Dev Plugin Part 3 - Enable remote ssh connection #2547

Open

Remote Dev Plugin Part 1 - Register the remote dev plugin and add utils #2545

Open

Remote Dev Plugin Part 2 - Add basic remote commands #2546

Open

sfc-gh-zzhu changed the title ~~bug fix and feature improvement~~ Remote Dev Plugin Part 3 - bug fix and feature improvement Aug 28, 2025

sfc-gh-zzhu requested a review from sfc-gh-dhung August 28, 2025 22:39

sfc-gh-zzhu marked this pull request as ready for review August 28, 2025 22:39

sfc-gh-zzhu requested review from a team as code owners August 28, 2025 22:39

sfc-gh-zzhu force-pushed the 08-22-bug_fix_and_feature_improvement branch 2 times, most recently from f2d260d to e7f252e Compare August 29, 2025 16:36

graphite-app bot reviewed Aug 29, 2025

View reviewed changes

sfc-gh-zzhu force-pushed the 08-22-bug_fix_and_feature_improvement branch from e7f252e to cf658b9 Compare August 29, 2025 17:24

sfc-gh-zzhu changed the title ~~Remote Dev Plugin Part 3 - bug fix and feature improvement~~ Remote Dev Plugin Part 4 - bug fix and feature improvement Aug 29, 2025

sfc-gh-zzhu force-pushed the 08-22-bug_fix_and_feature_improvement branch from cf658b9 to db13c9c Compare August 29, 2025 18:09

sfc-gh-dhung approved these changes Aug 29, 2025

View reviewed changes

sfc-gh-zzhu force-pushed the 08-22-bug_fix_and_feature_improvement branch from db13c9c to c9a1a5d Compare September 5, 2025 00:47

sfc-gh-zzhu force-pushed the 08-17-enable_remote_ssh_connection branch from 2610027 to 6d2eb90 Compare September 5, 2025 00:47

graphite-app bot reviewed Sep 5, 2025

View reviewed changes

sfc-gh-zzhu force-pushed the 08-22-bug_fix_and_feature_improvement branch from c9a1a5d to 784cd7a Compare September 5, 2025 20:30

graphite-app bot reviewed Sep 5, 2025

View reviewed changes

sfc-gh-zzhu force-pushed the 08-17-enable_remote_ssh_connection branch from 6d2eb90 to 77accb6 Compare September 15, 2025 05:30

sfc-gh-zzhu force-pushed the 08-22-bug_fix_and_feature_improvement branch 2 times, most recently from 582d471 to b9a97f7 Compare September 19, 2025 19:18

sfc-gh-zzhu force-pushed the 08-17-enable_remote_ssh_connection branch from 77accb6 to 85af513 Compare September 19, 2025 19:18

sfc-gh-zzhu mentioned this pull request Sep 29, 2025

Address feedbacks #2636

Draft

9 tasks

sfc-gh-zzhu added 2 commits October 16, 2025 14:37

bug fix and feature improvement

801f2d5

Use container-runtime 1.9.1 by default

01d14d8

sfc-gh-zzhu force-pushed the 08-22-bug_fix_and_feature_improvement branch from b9a97f7 to 01d14d8 Compare October 17, 2025 18:42

sfc-gh-zzhu force-pushed the 08-17-enable_remote_ssh_connection branch from 85af513 to 583ba5c Compare October 17, 2025 18:42

sfc-gh-zzhu mentioned this pull request Oct 17, 2025

Add default block volume for persistent storage #2660

Draft

9 tasks

-        subprocess.run([binary_name, "--folder-uri", folder_uri], check=False)
+        result = subprocess.run([binary_name, "--folder-uri", folder_uri], check=False)
+        if result.returncode != 0:
+            raise CliError(f"Failed to launch {binary_name}, exit code: {result.returncode}")


		# Check if user provided configuration options that would modify the service
		self._warn_about_config_changes(service_name, **config_options)

Comments

Conversation

sfc-gh-zzhu commented Aug 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-review checklist

Changes description

Uh oh!

sfc-gh-zzhu commented Aug 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

graphite-app bot Aug 29, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

graphite-app bot Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

graphite-app bot Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

graphite-app bot Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sfc-gh-zzhu commented Aug 28, 2025 •

edited

Loading

sfc-gh-zzhu commented Aug 28, 2025 •

edited

Loading