diff --git a/python/lightning_sdk/api/deployment_api.py b/python/lightning_sdk/api/deployment_api.py index 03deb1b6e..591797c1e 100644 --- a/python/lightning_sdk/api/deployment_api.py +++ b/python/lightning_sdk/api/deployment_api.py @@ -1115,6 +1115,10 @@ def _validate_remote_upload_path_target(*, client: Any, teamspace_id: str, remot return parsed = parse_lit_url(normalized) + if parsed.get("owner") is None and parsed.get("teamspace") is None: + # Relative form (lit:///) targets the current teamspace by definition. + return + parsed_teamspace = str(parsed.get("teamspace", "") or "").strip() if not parsed_teamspace: raise ValueError("remote_path lit URL must include a non-empty teamspace") diff --git a/python/lightning_sdk/cli/cp/__init__.py b/python/lightning_sdk/cli/cp/__init__.py index 0d1d744d5..ae3a83c1e 100644 --- a/python/lightning_sdk/cli/cp/__init__.py +++ b/python/lightning_sdk/cli/cp/__init__.py @@ -7,48 +7,24 @@ from lightning_sdk.filesystem import Filesystem -def parse_lit_url(url: str) -> str: - """Parse lit:// URL and extract resource type.""" - if not url.startswith("lit://"): - raise ValueError("URL must start with 'lit://'") - - path = url.split("://")[-1].split("/") - if len(path) < 3 or not path[2]: - raise ValueError("Invalid lit URL format. Expected 'lit:////'") - return path[2].lower() - - -def _canonicalize_lit_resource_type(url: str) -> str: - """Normalize the lit:// resource type segment to its canonical lowercase form.""" - parse_lit_url(url) - path = url.split("://", maxsplit=1)[-1].split("/") - path[2] = path[2].lower() - return "lit://" + "/".join(path) - - def route_cp_operation(source: str, destination: Optional[str], **options: Any) -> None: - """Route copy operation based on URL structure.""" + """Route copy operation based on URL structure. + + Drive paths are passed through untouched — the server owns their validation + (resource types, case, existence). + """ if destination is None: raise ValueError("Destination path must be provided.") source_is_lit = source.startswith("lit://") dest_is_lit = destination.startswith("lit://") - if source_is_lit: - source = _canonicalize_lit_resource_type(source) - if dest_is_lit: - destination = _canonicalize_lit_resource_type(destination) - if source_is_lit and dest_is_lit: raise ValueError("Cannot copy between two remote URLs. One path must be local.") if not source_is_lit and not dest_is_lit: raise ValueError("At least one path must be a lit://") - # Every resource type is a path in the teamspace drive, passed through - # for the server to resolve. This validates the URL shape up front. - parse_lit_url(source if source_is_lit else destination) - return Filesystem().copy( source=source, destination=destination, diff --git a/python/lightning_sdk/cli/cp/completion.py b/python/lightning_sdk/cli/cp/completion.py index 06a8f1aab..fcff58c12 100644 --- a/python/lightning_sdk/cli/cp/completion.py +++ b/python/lightning_sdk/cli/cp/completion.py @@ -101,9 +101,23 @@ def _safe_remote_completions(incomplete: str) -> list[CompletionItem]: def _complete_remote_path(incomplete: str) -> list[CompletionItem]: parts = incomplete[len(_LIT_PREFIX) :].split("/") + if parts[0] == "" and len(parts) > 1: + # Relative form lit:/// — complete from the current teamspace's drive. + from lightning_sdk.utils.resolve import _resolve_teamspace + + teamspace = _resolve_teamspace(teamspace=None, org=None, user=None) + if teamspace is None: + return [] + parent = "/".join(parts[1:-1]) + entries = FilesystemApi().list_files(teamspace.id, parent, recursive=False) + return _complete_tree_entries(incomplete, entries) + if len(parts) == 1: owner_names = _accessible_teamspaces() - return _complete_values(incomplete, (f"{_LIT_PREFIX}{owner}/" for owner in owner_names)) + return _complete_values( + incomplete, + (f"{_LIT_PREFIX}{owner}/" for owner in owner_names), + ) + _complete_values(incomplete, [f"{_LIT_PREFIX}/"]) owner = parts[0] if len(parts) == 2: diff --git a/python/lightning_sdk/cli/groups.py b/python/lightning_sdk/cli/groups.py index 194561b6e..93e7c1233 100644 --- a/python/lightning_sdk/cli/groups.py +++ b/python/lightning_sdk/cli/groups.py @@ -206,19 +206,21 @@ def dataset() -> None: def cp(_ctx: click.Context) -> None: """Copy between local, Studios, Drive. - Every lit:// URL must include a resource root right after the teamspace that - tells Lightning where to send the file. There is no default - a URL without - a root will fail. + Every lit:// URL must include a resource root at the start of its drive path + that tells Lightning where to send the file. There is no default - a URL + without a root will fail. URL formats: Studios: lit:////studios// Teamspace drives: lit:////uploads/ + Current teamspace (relative): lit://// Examples: lightning cp source.txt lit:////studios//destination.txt lightning cp -r source_folder/ lit:////studios//destination_folder/ lightning cp source.txt lit:////uploads/destination.txt - lightning cp -r source_folder/ lit:////uploads/destination_folder/ + lightning cp source.txt lit:///uploads/destination.txt + lightning cp -r source_folder/ lit:///studios//destination_folder/ """ @@ -239,9 +241,11 @@ def edit(_ctx: click.Context) -> None: URL formats: Studios: lit:////studios// Teamspace drives: lit:////uploads/ + Current teamspace (relative): lit://// Examples: lightning edit lit:////studios//notes.txt + lightning edit lit:///studios//notes.txt lightning edit lit:////uploads/config.yaml --editor "code -w" """ diff --git a/python/lightning_sdk/cli/legacy_redirects.py b/python/lightning_sdk/cli/legacy_redirects.py index fc5a2b6af..6fd6eb4a8 100644 --- a/python/lightning_sdk/cli/legacy_redirects.py +++ b/python/lightning_sdk/cli/legacy_redirects.py @@ -163,3 +163,35 @@ def _deprecated_get_help(ctx: click.Context) -> str: def build_hidden_alias_group(name: str, target_group: click.Group) -> HiddenAliasGroup: return HiddenAliasGroup(name=name, target_group=target_group) + + +def mark_deprecated_command(cmd: click.Command, replacement: str, detail: str | None = None) -> click.Command: + """Mark a still-functional command as deprecated in favor of ``replacement``. + + Unlike :class:`DeprecatedForwardCommand`, the command keeps its own behavior; a + warning is shown on every invocation and prepended to its help text. ``detail`` + is appended to the warning — use it when switching commands takes more than a + rename (e.g. a different URL format). + """ + dynamic_cmd = cast(Any, cmd) + if getattr(dynamic_cmd, "_lightning_deprecation_wrapped", False): + return cmd + + original_invoke = cmd.invoke + original_get_help = cmd.get_help + + def _warning(ctx: click.Context) -> str: + message = _format_deprecation_warning(ctx.command_path, replacement) + return f"{message} {detail}" if detail else message + + def _deprecated_invoke(ctx: click.Context) -> object: + click.secho(_warning(ctx), fg="yellow", err=True) + return original_invoke(ctx) + + def _deprecated_get_help(ctx: click.Context) -> str: + return f"{_warning(ctx)}\n\n{original_get_help(ctx)}" + + dynamic_cmd.invoke = _deprecated_invoke + dynamic_cmd.get_help = _deprecated_get_help + dynamic_cmd._lightning_deprecation_wrapped = True + return cmd diff --git a/python/lightning_sdk/cli/ls/__init__.py b/python/lightning_sdk/cli/ls/__init__.py index c98589185..594a8530c 100644 --- a/python/lightning_sdk/cli/ls/__init__.py +++ b/python/lightning_sdk/cli/ls/__init__.py @@ -5,10 +5,9 @@ from lightning_sdk.api.filesystem_api import FilesystemApi from lightning_sdk.api.utils import _tree_path_info from lightning_sdk.cli.cp.completion import complete_remote_path -from lightning_sdk.cli.utils.filesystem import resolve_teamspace +from lightning_sdk.cli.utils.filesystem import resolve_lit_url from lightning_sdk.cli.utils.json_output import echo_json from lightning_sdk.cli.utils.logging import LightningCommand -from lightning_sdk.utils.filesystem import parse_lit_url @click.command("ls", cls=LightningCommand) @@ -18,12 +17,14 @@ def ls(path: str, recursive: bool = False, as_json: bool = False) -> None: """List contents of a teamspace drive directory. - PATH: Drive path in the format lit:////. + PATH: Drive path in the format lit:////, + or lit:/// for the current teamspace. The teamspace root lists the drive's top-level folders (studios, uploads, ...). Examples: lightning ls lit://// lightning ls lit:////artifacts/reports + lightning ls lit:///artifacts/reports lightning ls -r lit:////artifacts/reports lightning ls --json lit:////artifacts/reports @@ -35,9 +36,8 @@ def ls_impl(path: str, recursive: bool = False, as_json: bool = False) -> None: if not path.startswith("lit://"): raise ValueError("Path must be a drive path starting with 'lit://'.") - path_result = parse_lit_url(path) - remote_path = (path_result["destination"] or "").strip("/") - selected_teamspace = resolve_teamspace(path_result["teamspace"], path_result["owner"]) + selected_teamspace, remote_path = resolve_lit_url(path) + remote_path = remote_path.strip("/") filesystem_api = FilesystemApi() diff --git a/python/lightning_sdk/cli/rm/__init__.py b/python/lightning_sdk/cli/rm/__init__.py index 90118bf13..fcefb1c70 100644 --- a/python/lightning_sdk/cli/rm/__init__.py +++ b/python/lightning_sdk/cli/rm/__init__.py @@ -14,10 +14,12 @@ def rm(path: str, recursive: bool = False, force: bool = False) -> None: """Remove a file or directory from a teamspace drive. - PATH: Drive path to remove, in the format lit:////. + PATH: Drive path to remove, in the format lit:////, + or lit:/// for the current teamspace. Examples: lightning rm lit:////uploads/file.txt + lightning rm lit:///uploads/file.txt lightning rm -r lit:////artifacts/reports/ """ diff --git a/python/lightning_sdk/cli/studio/__init__.py b/python/lightning_sdk/cli/studio/__init__.py index 1d4e30902..6d7f49d5c 100644 --- a/python/lightning_sdk/cli/studio/__init__.py +++ b/python/lightning_sdk/cli/studio/__init__.py @@ -5,6 +5,7 @@ def register_commands(group: click.Group) -> None: """Register studio commands with the given group.""" + from lightning_sdk.cli.legacy_redirects import mark_deprecated_command from lightning_sdk.cli.studio.connect import connect_studio from lightning_sdk.cli.studio.cp import cp_studio_file from lightning_sdk.cli.studio.create import create_studio @@ -36,7 +37,13 @@ def register_commands(group: click.Group) -> None: group.add_command(stop_studio) group.add_command(switch_studio) group.add_command(connect_studio) - group.add_command(cp_studio_file) - group.add_command(ls_studio) - group.add_command(rm_studio_file) + # The replacements take full drive URLs, not this group's short studio forms, so the + # warning spells out the conversion — a bare command rename could target a different drive. + url_note = ( + "Note the URL format differs: use lit:////studios//, " + "or lit:///studios// for the current teamspace." + ) + group.add_command(mark_deprecated_command(cp_studio_file, "lightning cp", detail=url_note)) + group.add_command(mark_deprecated_command(ls_studio, "lightning ls", detail=url_note)) + group.add_command(mark_deprecated_command(rm_studio_file, "lightning rm", detail=url_note)) group.add_command(open_studio, name="open") diff --git a/python/lightning_sdk/cli/studio/cp.py b/python/lightning_sdk/cli/studio/cp.py index 28b9428b3..06c0f8554 100644 --- a/python/lightning_sdk/cli/studio/cp.py +++ b/python/lightning_sdk/cli/studio/cp.py @@ -6,7 +6,7 @@ from rich.console import Console from lightning_sdk.api.utils import _get_cloud_url -from lightning_sdk.cli.utils.filesystem import parse_studio_path, resolve_studio +from lightning_sdk.cli.studio.paths import parse_studio_path, resolve_studio from lightning_sdk.cli.utils.logging import LightningCommand from lightning_sdk.filesystem import Filesystem from lightning_sdk.studio import Studio @@ -19,13 +19,16 @@ def cp_studio_file(source: str, destination: str, recursive: bool = False) -> None: """Copy a Studio file. - SOURCE: Source file to copy from. For Studio files, use the format lit:////studios//. + SOURCE: Source file to copy from. For Studio files, use the format + lit:////studios//, or + lit:///studios// for the current teamspace. - DESTINATION: Destination file to copy to. For Studio files, use the format lit:////studios//. + DESTINATION: Destination file to copy to. For Studio files, use the same formats. Example: lightning studio cp source.txt lit:////studios//destination.txt - lightning studio cp -r source_folder/ lit:////studios//destination_folder/ + lightning studio cp source.txt lit:///studios//destination.txt + lightning studio cp -r source_folder/ lit:///studios//destination_folder/ """ return cp_impl(source=source, destination=destination, recursive=recursive) @@ -59,7 +62,8 @@ def _resolve_drive_url(studio_path: str) -> Tuple[Studio, str]: """Resolve a studio lit URL to the studio and its fully-qualified drive URL. Unlike the main ``lightning cp`` URLs, studio paths may omit the owner or - the owner and teamspace, which then resolve from the configured defaults. + the owner and teamspace, which then resolve from the configured defaults; + the relative form (``lit:///studios/...``) resolves the same way. """ parsed = parse_studio_path(studio_path) studio = resolve_studio(parsed["studio"], parsed["teamspace"], parsed["owner"]) diff --git a/python/lightning_sdk/cli/studio/ls.py b/python/lightning_sdk/cli/studio/ls.py index 15254460d..e4f434d6a 100644 --- a/python/lightning_sdk/cli/studio/ls.py +++ b/python/lightning_sdk/cli/studio/ls.py @@ -2,7 +2,7 @@ import rich_click as click -from lightning_sdk.cli.utils.filesystem import parse_studio_path, resolve_studio +from lightning_sdk.cli.studio.paths import parse_studio_path, resolve_studio from lightning_sdk.cli.utils.logging import LightningCommand @@ -11,11 +11,12 @@ def ls_studio(path: str) -> None: """List contents of a directory in Studio. - PATH: Studio path in the format - lit:////studios// + PATH: Studio path in the format lit:////studios//, + or lit:///studios// for the current teamspace. Example: lightning studio ls lit:////studios//data + lightning studio ls lit:///studios//data """ return ls_impl(path=path) diff --git a/python/lightning_sdk/cli/studio/paths.py b/python/lightning_sdk/cli/studio/paths.py new file mode 100644 index 000000000..7cf4e9c13 --- /dev/null +++ b/python/lightning_sdk/cli/studio/paths.py @@ -0,0 +1,89 @@ +"""Path parsing and resolution for the deprecated ``lightning studio cp/ls/rm`` commands. + +The main drive commands (``lightning cp/ls/rm``) go through +:func:`lightning_sdk.utils.filesystem.parse_lit_url` instead; the parser here +additionally accepts the studio commands' legacy short forms. +""" + +from typing import Optional + +from lightning_sdk.cli.utils.resource_resolution import join_teamspace_slug +from lightning_sdk.cli.utils.resource_resolution import resolve_studio as resolve_cli_studio +from lightning_sdk.cli.utils.resource_resolution import resolve_teamspace as resolve_cli_teamspace +from lightning_sdk.studio import Studio +from lightning_sdk.utils.filesystem import PathResult + + +def parse_studio_path(studio_path: str) -> PathResult: + """Parse a studio ``lit://`` URL into owner, teamspace, studio, and destination. + + Accepts ``lit://[owner/][teamspace/]studios//``, the short form + ``lit:///``, and the relative form ``lit:///studios//`` + (a studio in the current teamspace). Omitted owner and teamspace resolve to the + configured defaults. + """ + prefix = "lit://" + has_prefix = studio_path.startswith(prefix) + path_string = studio_path[len(prefix) :] if has_prefix else studio_path + if not path_string: + raise ValueError("Studio path cannot be empty after prefix") + + result: PathResult = {"owner": None, "teamspace": None, "studio": None, "destination": None} + + relative = path_string.startswith("/") + if relative: + # Relative form: lit:///... targets the current teamspace, which is also + # what the short forms below resolve to when owner/teamspace are omitted. + # Only the lit:/// spelling means that — a bare absolute path is a + # local-path mistake, not a studio path. + path_string = path_string[1:] + if not has_prefix or path_string.startswith("/") or (path_string and not path_string.startswith("studios/")): + raise ValueError( + f"Invalid studio path {studio_path!r}. Expected 'lit:////studios//' " + "or 'lit:///studios//' for the current teamspace." + ) + if not path_string: + raise ValueError("Studio path cannot be empty after prefix") + + if relative: + # lit:///studios// — the leading root has no owner/teamspace + # before it, so the "/studios/" split below would not match it. + path_parts = path_string[len("studios/") :].split("/") + + elif "/studios/" in path_string: + prefix_part, suffix_part = path_string.split("/studios/", 1) + + # org and teamspace + if prefix_part: + org_ts_components = prefix_part.split("/") + if len(org_ts_components) == 2: + result["owner"], result["teamspace"] = org_ts_components + elif len(org_ts_components) == 1: + result["teamspace"] = org_ts_components[0] + else: + raise ValueError(f"Invalid format: '{prefix_part}'") + + # studio and destination + path_parts = suffix_part.split("/") + + else: + # studio and destination + path_parts = path_string.split("/") + + if not path_parts or not path_parts[0]: + raise ValueError("Invalid: Missing studio name.") + + if len(path_parts) == 1: + raise ValueError( + "Invalid: Invalid studio path. To refer to the studio root, add a trailing '/' (e.g., 'lit:////studios//')" + ) + + result["studio"] = path_parts[0] + result["destination"] = "/".join(path_parts[1:]) + + return result + + +def resolve_studio(studio_name: Optional[str], teamspace: Optional[str], owner: Optional[str]) -> Studio: + resolved_teamspace = resolve_cli_teamspace(join_teamspace_slug(owner, teamspace)) + return resolve_cli_studio(studio_name, resolved_teamspace) diff --git a/python/lightning_sdk/cli/studio/rm.py b/python/lightning_sdk/cli/studio/rm.py index 390aaf1a0..f3c1fe456 100644 --- a/python/lightning_sdk/cli/studio/rm.py +++ b/python/lightning_sdk/cli/studio/rm.py @@ -3,7 +3,7 @@ import rich_click as click from rich.console import Console -from lightning_sdk.cli.utils.filesystem import parse_studio_path, resolve_studio +from lightning_sdk.cli.studio.paths import parse_studio_path, resolve_studio from lightning_sdk.cli.utils.logging import LightningCommand from lightning_sdk.studio import Studio @@ -15,11 +15,12 @@ def rm_studio_file(path: str, recursive: bool = False, force: bool = False) -> None: """Remove a Studio file or directory. - PATH: Studio path to remove. Use the format lit:////studios//. + PATH: Studio path to remove. Use the format lit:////studios//, + or lit:///studios// for the current teamspace. Example: lightning studio rm lit:////studios//file.txt - lightning studio rm -r lit:////studios//folder/ + lightning studio rm -r lit:///studios//folder/ """ return rm_impl(path=path, recursive=recursive, force=force) diff --git a/python/lightning_sdk/cli/utils/filesystem.py b/python/lightning_sdk/cli/utils/filesystem.py index dad9af047..c4ed94657 100644 --- a/python/lightning_sdk/cli/utils/filesystem.py +++ b/python/lightning_sdk/cli/utils/filesystem.py @@ -1,65 +1,21 @@ -from typing import Optional, TypedDict +from typing import Optional, Tuple from lightning_sdk.cli.utils.resource_resolution import join_teamspace_slug -from lightning_sdk.cli.utils.resource_resolution import resolve_studio as resolve_cli_studio from lightning_sdk.cli.utils.resource_resolution import resolve_teamspace as resolve_cli_teamspace -from lightning_sdk.studio import Studio from lightning_sdk.teamspace import Teamspace - - -class PathResult(TypedDict): - owner: Optional[str] - teamspace: Optional[str] - studio: Optional[str] - destination: Optional[str] - - -def parse_studio_path(studio_path: str) -> PathResult: - prefix = "lit://" - path_string = studio_path[len(prefix) :] if studio_path.startswith(prefix) else studio_path - if not path_string: - raise ValueError("Studio path cannot be empty after prefix") - - result: PathResult = {"owner": None, "teamspace": None, "studio": None, "destination": None} - - if "/studios/" in path_string: - prefix_part, suffix_part = path_string.split("/studios/", 1) - - # org and teamspace - if prefix_part: - org_ts_components = prefix_part.split("/") - if len(org_ts_components) == 2: - result["owner"], result["teamspace"] = org_ts_components - elif len(org_ts_components) == 1: - result["teamspace"] = org_ts_components[0] - else: - raise ValueError(f"Invalid format: '{prefix_part}'") - - # studio and destination - path_parts = suffix_part.split("/") - - else: - # studio and destination - path_parts = path_string.split("/") - - if not path_parts: - raise ValueError("Invalid: Missing studio name.") - - if len(path_parts) == 1: - raise ValueError( - "Invalid: Invalid studio path. To refer to the studio root, add a trailing '/' (e.g., 'lit:////studios//')" - ) - - result["studio"] = path_parts[0] - result["destination"] = "/".join(path_parts[1:]) - - return result +from lightning_sdk.utils.filesystem import parse_lit_url def resolve_teamspace(teamspace: Optional[str], owner: Optional[str]) -> Teamspace: return resolve_cli_teamspace(join_teamspace_slug(owner, teamspace)) -def resolve_studio(studio_name: Optional[str], teamspace: Optional[str], owner: Optional[str]) -> Studio: - resolved_teamspace = resolve_teamspace(teamspace, owner) - return resolve_cli_studio(studio_name, resolved_teamspace) +def resolve_lit_url(url: str) -> Tuple[Teamspace, str]: + """Parse a ``lit://`` URL and resolve its teamspace in one step. + + Returns the resolved :class:`Teamspace` and the drive path within it + (``""`` for the teamspace root). The relative form (``lit:///``) + resolves to the current teamspace. + """ + parsed = parse_lit_url(url) + return resolve_teamspace(parsed["teamspace"], parsed["owner"]), parsed["destination"] or "" diff --git a/python/lightning_sdk/filesystem.py b/python/lightning_sdk/filesystem.py index f88eeb4bd..50c282459 100644 --- a/python/lightning_sdk/filesystem.py +++ b/python/lightning_sdk/filesystem.py @@ -6,9 +6,8 @@ from lightning_sdk.api.filesystem_api import FilesystemApi from lightning_sdk.api.utils import _RemoteApiError, _tree_path_info -from lightning_sdk.cli.utils.filesystem import resolve_teamspace +from lightning_sdk.cli.utils.filesystem import resolve_lit_url from lightning_sdk.teamspace import Teamspace -from lightning_sdk.utils.filesystem import parse_lit_url from lightning_sdk.utils.logging import TrackCallsMeta logger = logging.getLogger(__name__) @@ -30,14 +29,13 @@ def listdir(self, uri: str) -> List[str]: """List the immediate children of a remote directory. Args: - uri: Remote path in ``lit://[owner/][teamspace/]destination`` format. + uri: Remote path in ``lit:////`` format, or + ``lit:///`` for the current teamspace. Returns: List[str]: Basenames of the entries directly inside the given directory. """ - path_result = parse_lit_url(uri) - remote_path = path_result["destination"] or "" - selected_teamspace = resolve_teamspace(path_result["teamspace"], path_result["owner"]) + selected_teamspace, remote_path = resolve_lit_url(uri) output = self._filesystem_api.list_files(teamspace_id=selected_teamspace.id, path=remote_path, recursive=False) return [os.path.basename(item["path"]) for item in output] @@ -45,16 +43,15 @@ def walk(self, url: str) -> Generator[Tuple[str, List[str], List[str]], None, No """Recursively walk a remote directory tree, yielding ``(dirpath, subdirs, files)`` tuples. Args: - url: Remote path in ``lit://[owner/][teamspace/]destination`` format. + url: Remote path in ``lit:////`` format, or + ``lit:///`` for the current teamspace. Returns: Generator[Tuple[str, List[str], List[str]], None, None]: Each tuple contains the current directory path, a list of its immediate subdirectory names, and a list of its immediate file names — mirroring the behaviour of :func:`os.walk`. """ - path_result = parse_lit_url(url) - remote_path = path_result["destination"] or "" - selected_teamspace = resolve_teamspace(path_result["teamspace"], path_result["owner"]) + selected_teamspace, remote_path = resolve_lit_url(url) output = self._filesystem_api.list_files(teamspace_id=selected_teamspace.id, path=remote_path, recursive=True) dirs: dict[str, list[str]] = {} @@ -81,7 +78,8 @@ def rm(self, path: str, recursive: bool = False) -> None: """Remove a file or directory from the teamspace drive. Args: - path: Remote path in ``lit:////`` format. + path: Remote path in ``lit:////`` format, or + ``lit:///`` for the current teamspace. recursive: Required to remove a directory and everything under it. Raises: @@ -89,11 +87,10 @@ def rm(self, path: str, recursive: bool = False) -> None: ValueError: If the path is a directory and ``recursive`` is ``False``, or the path has no file or directory component. """ - path_result = parse_lit_url(path) - remote_path = (path_result["destination"] or "").strip("/") + selected_teamspace, remote_path = resolve_lit_url(path) + remote_path = remote_path.strip("/") if not remote_path: raise ValueError("Refusing to remove the teamspace root; pass a file or directory path.") - selected_teamspace = resolve_teamspace(path_result["teamspace"], path_result["owner"]) def list_entries(folder: str) -> List[dict]: return self._filesystem_api.list_files(teamspace_id=selected_teamspace.id, path=folder, recursive=False) @@ -151,11 +148,8 @@ def copy( if not source_is_lit and not dest_is_lit: raise ValueError("At least one path must be a lit://") - path_result = parse_lit_url(source if source_is_lit else destination) - remote_path = path_result["destination"] or "" + selected_teamspace, remote_path = resolve_lit_url(source if source_is_lit else destination) local_path = destination if source_is_lit else source - - selected_teamspace = resolve_teamspace(path_result["teamspace"], path_result["owner"]) if source_is_lit: # download parent = os.path.dirname(remote_path.strip("/")) diff --git a/python/lightning_sdk/utils/filesystem.py b/python/lightning_sdk/utils/filesystem.py index c42d23818..89cce91ff 100644 --- a/python/lightning_sdk/utils/filesystem.py +++ b/python/lightning_sdk/utils/filesystem.py @@ -8,31 +8,56 @@ class PathResult(TypedDict): destination: Optional[str] +_SHORT_FORM_ERROR = ( + "Invalid lit URL {url!r}. Expected 'lit:///[/]', or 'lit:///' " + "for a path in the current teamspace." +) + + def parse_lit_url(url: str) -> PathResult: """Parse a ``lit://`` URL into its owner, teamspace, and destination components. - Args: - url: A URL in ``lit://owner/teamspace[/destination]`` format, or a bare - ``owner/teamspace[/destination]`` path without the prefix. + Two forms are supported: + + - Long form: ``lit:///[/]``. + - Relative form: ``lit:///`` — a path in the current teamspace. + ``owner`` and ``teamspace`` are returned as ``None`` and resolve to the + configured defaults (or the Studio's teamspace when running in one). + + A bare ``owner/teamspace[/destination]`` path without the prefix is also accepted; + a bare absolute path (e.g. ``/tmp/file``) is rejected, since only the ``lit:///`` + spelling means "the current teamspace" — never a stray local path. Returns: PathResult: A dict with ``owner``, ``teamspace``, ``studio``, and ``destination`` keys. ``studio`` is always ``None`` (reserved for future use). Raises: - ValueError: If the path is empty after stripping the prefix, or if fewer than - two path components are present. + ValueError: If the path is empty after stripping the prefix, or if it is neither + a relative form nor has the owner and teamspace components. """ prefix = "lit://" - path_string = url[len(prefix) :] if url.startswith(prefix) else url + has_prefix = url.startswith(prefix) + path_string = url[len(prefix) :] if has_prefix else url if not path_string: raise ValueError("Teamspace path cannot be empty after prefix") result: PathResult = {"owner": None, "teamspace": None, "studio": None, "destination": None} + if path_string.startswith("/"): + # Relative form: lit:/// targets the current teamspace. + # An empty destination is the teamspace root, as in the long form. + # A bare absolute path is a local-path mistake, not a drive path — callers + # like Filesystem.rm would otherwise turn it into a remote operation. + destination = path_string[1:] + if not has_prefix or destination.startswith("/"): + raise ValueError(_SHORT_FORM_ERROR.format(url=url)) + result["destination"] = destination + return result + path_parts = path_string.split("/") if len(path_parts) < 2: - raise ValueError("Invalid lit URL format. Expected at least 'lit:///'") + raise ValueError(_SHORT_FORM_ERROR.format(url=url)) # get teamspace result["owner"], result["teamspace"] = path_parts[0], path_parts[1] diff --git a/python/tests/cli/cp/test_completion.py b/python/tests/cli/cp/test_completion.py index f8a5118cf..f696f9b80 100644 --- a/python/tests/cli/cp/test_completion.py +++ b/python/tests/cli/cp/test_completion.py @@ -226,3 +226,23 @@ def test_remote_only_completion_walks_lit_paths(_accessible_teamspaces): parameter = next(parameter for parameter in cp.params if parameter.name == "source") assert _values(complete_remote_path(ctx, parameter, "lit://a")) == ["lit://acme/"] + + +@patch("lightning_sdk.utils.resolve._resolve_teamspace") +@patch("lightning_sdk.cli.cp.completion.FilesystemApi") +def test_remote_completion_relative_form_uses_current_teamspace(filesystem_api, resolve_teamspace): + resolve_teamspace.return_value = SimpleNamespace(id="project-1") + filesystem_api.return_value.list_files.return_value = [ + {"path": "artifacts", "type": "tree"}, + {"path": "uploads", "type": "tree"}, + ] + + items = _complete_argument("source", "lit:///u") + + assert _values(items) == ["lit:///uploads/"] + filesystem_api.return_value.list_files.assert_called_once_with("project-1", "", recursive=False) + + +@patch("lightning_sdk.utils.resolve._resolve_teamspace", return_value=None) +def test_remote_completion_relative_form_without_current_teamspace_is_empty(_resolve_teamspace): + assert _complete_argument("source", "lit:///u") == [] diff --git a/python/tests/cli/cp/test_cp_download.py b/python/tests/cli/cp/test_cp_download.py index 468c33e3d..04592a141 100644 --- a/python/tests/cli/cp/test_cp_download.py +++ b/python/tests/cli/cp/test_cp_download.py @@ -36,14 +36,24 @@ def test_route_cp_raises_if_both_local(): ) -def test_route_cp_raises_for_invalid_short_lit_url(): - """Test that malformed short lit URLs still raise a clear ValueError.""" - with pytest.raises(ValueError, match="Invalid lit URL format"): +def test_route_cp_passes_paths_through_for_server_validation(): + """Drive paths are not validated client-side — the server owns that.""" + mock_fs = MagicMock() + + with patch("lightning_sdk.cli.cp.Filesystem", return_value=mock_fs): route_cp_operation( source="lit://my-org/my-teamspace", destination="/local/model.ckpt", ) + mock_fs.copy.assert_called_once_with( + source="lit://my-org/my-teamspace", + destination="/local/model.ckpt", + recursive=False, + progress_bar=True, + cloud_account=None, + ) + def test_route_cp_raises_if_destination_is_missing(): """Test that missing destination raises a clear ValueError.""" @@ -175,8 +185,8 @@ def test_route_cp_download_passes_paths_with_uploads_segment_through(): ) -def test_route_cp_download_canonicalizes_mixed_case_resource_type(): - """Test that mixed-case remote resource types are canonicalized before download dispatch.""" +def test_route_cp_download_passes_mixed_case_resource_type_through(): + """Mixed-case resource types are passed through — the server resolves them case-insensitively.""" mock_fs = MagicMock() source = "lit://my-org/my-teamspace/Lightning_Storage/my-storage/data/model.ckpt" @@ -188,7 +198,7 @@ def test_route_cp_download_canonicalizes_mixed_case_resource_type(): ) mock_fs.copy.assert_called_once_with( - source="lit://my-org/my-teamspace/lightning_storage/my-storage/data/model.ckpt", + source="lit://my-org/my-teamspace/Lightning_Storage/my-storage/data/model.ckpt", destination="/local/model.ckpt", recursive=False, progress_bar=True, @@ -196,6 +206,26 @@ def test_route_cp_download_canonicalizes_mixed_case_resource_type(): ) +def test_route_cp_relative_url_delegates(): + """Relative lit:/// URLs are accepted and passed through to the copy.""" + mock_fs = MagicMock() + + with patch("lightning_sdk.cli.cp.Filesystem", return_value=mock_fs): + route_cp_operation( + source="/local/model.ckpt", + destination="lit:///Uploads/model.ckpt", + recursive=False, + ) + + mock_fs.copy.assert_called_once_with( + source="/local/model.ckpt", + destination="lit:///Uploads/model.ckpt", + recursive=False, + progress_bar=True, + cloud_account=None, + ) + + def test_route_cp_lightning_storage_upload(): """Test that lightning_storage upload routes to Filesystem.copy.""" mock_fs = MagicMock() @@ -216,8 +246,8 @@ def test_route_cp_lightning_storage_upload(): ) -def test_route_cp_lightning_storage_upload_canonicalizes_mixed_case_resource_type(): - """Test that mixed-case upload resource types are canonicalized before Filesystem.copy.""" +def test_route_cp_lightning_storage_upload_passes_mixed_case_resource_type_through(): + """Mixed-case upload resource types are passed through — the server resolves them.""" mock_fs = MagicMock() with patch("lightning_sdk.cli.cp.Filesystem", return_value=mock_fs): @@ -229,7 +259,7 @@ def test_route_cp_lightning_storage_upload_canonicalizes_mixed_case_resource_typ mock_fs.copy.assert_called_once_with( source="/local/model.ckpt", - destination="lit://my-org/my-teamspace/lightning_storage/my-storage/data/model.ckpt", + destination="lit://my-org/my-teamspace/Lightning_Storage/my-storage/data/model.ckpt", recursive=False, progress_bar=True, cloud_account=None, diff --git a/python/tests/cli/cp/test_cp_parse.py b/python/tests/cli/cp/test_cp_parse.py deleted file mode 100644 index faf2ea514..000000000 --- a/python/tests/cli/cp/test_cp_parse.py +++ /dev/null @@ -1,29 +0,0 @@ -import pytest - -from lightning_sdk.cli.cp import parse_lit_url - - -def test_parse_lit_url_missing_scheme(): - with pytest.raises(ValueError, match="URL must start with 'lit://"): - parse_lit_url("teamspace/org/studios/my-studio") - - -def test_parse_lit_url_studios(): - assert parse_lit_url("lit://org/teamspace/studios/my-studio") == "studios" - - -def test_parse_lit_url_uploads(): - assert parse_lit_url("lit://org/teamspace/uploads/my-file") == "uploads" - - -def test_parse_lit_url_s3_folders(): - assert parse_lit_url("lit://org/teamspace/s3_folders/my-folder") == "s3_folders" - - -def test_parse_lit_url_lightning_storage(): - assert parse_lit_url("lit://org/teamspace/lightning_storage/my-data") == "lightning_storage" - - -def test_parse_lit_url_missing_resource_type(): - with pytest.raises(ValueError, match="Invalid lit URL format"): - parse_lit_url("lit://org/teamspace") diff --git a/python/tests/cli/ls/test_ls.py b/python/tests/cli/ls/test_ls.py index bb65005c4..c6e1c26bc 100644 --- a/python/tests/cli/ls/test_ls.py +++ b/python/tests/cli/ls/test_ls.py @@ -32,7 +32,7 @@ def list_files(teamspace_id, path, recursive): {"path": "summary.html", "type": "blob", "size": 10}, ] - with mock.patch("lightning_sdk.cli.ls.resolve_teamspace", return_value=_fake_teamspace()), mock.patch( + with mock.patch("lightning_sdk.cli.utils.filesystem.resolve_teamspace", return_value=_fake_teamspace()), mock.patch( "lightning_sdk.cli.ls.FilesystemApi" ) as mock_api_cls: mock_api_cls.return_value.list_files.side_effect = list_files @@ -45,7 +45,7 @@ def test_ls_file_prints_its_path(capsys) -> None: def list_files(teamspace_id, path, recursive): return [{"path": "summary.html", "type": "blob", "size": 10}] - with mock.patch("lightning_sdk.cli.ls.resolve_teamspace", return_value=_fake_teamspace()), mock.patch( + with mock.patch("lightning_sdk.cli.utils.filesystem.resolve_teamspace", return_value=_fake_teamspace()), mock.patch( "lightning_sdk.cli.ls.FilesystemApi" ) as mock_api_cls: mock_api_cls.return_value.list_files.side_effect = list_files @@ -55,7 +55,7 @@ def list_files(teamspace_id, path, recursive): def test_ls_missing_path_raises() -> None: - with mock.patch("lightning_sdk.cli.ls.resolve_teamspace", return_value=_fake_teamspace()), mock.patch( + with mock.patch("lightning_sdk.cli.utils.filesystem.resolve_teamspace", return_value=_fake_teamspace()), mock.patch( "lightning_sdk.cli.ls.FilesystemApi" ) as mock_api_cls: mock_api_cls.return_value.list_files.return_value = [] @@ -79,7 +79,7 @@ def list_files(teamspace_id, path, recursive): {"path": "summary.html", "type": "blob", "size": 10, "clusterId": "cluster-a"}, ] - with mock.patch("lightning_sdk.cli.ls.resolve_teamspace", return_value=_fake_teamspace()), mock.patch( + with mock.patch("lightning_sdk.cli.utils.filesystem.resolve_teamspace", return_value=_fake_teamspace()), mock.patch( "lightning_sdk.cli.ls.FilesystemApi" ) as mock_api_cls: mock_api_cls.return_value.list_files.side_effect = list_files @@ -98,7 +98,7 @@ def test_ls_json_file_outputs_its_entry(capsys) -> None: def list_files(teamspace_id, path, recursive): return [{"path": "summary.html", "type": "blob", "size": 10, "clusterId": "cluster-a"}] - with mock.patch("lightning_sdk.cli.ls.resolve_teamspace", return_value=_fake_teamspace()), mock.patch( + with mock.patch("lightning_sdk.cli.utils.filesystem.resolve_teamspace", return_value=_fake_teamspace()), mock.patch( "lightning_sdk.cli.ls.FilesystemApi" ) as mock_api_cls: mock_api_cls.return_value.list_files.side_effect = list_files @@ -120,7 +120,7 @@ def list_files(teamspace_id, path, recursive): ] raise AssertionError("expected a recursive listing") - with mock.patch("lightning_sdk.cli.ls.resolve_teamspace", return_value=_fake_teamspace()), mock.patch( + with mock.patch("lightning_sdk.cli.utils.filesystem.resolve_teamspace", return_value=_fake_teamspace()), mock.patch( "lightning_sdk.cli.ls.FilesystemApi" ) as mock_api_cls: mock_api_cls.return_value.list_files.side_effect = list_files diff --git a/python/tests/cli/studio/test_cp.py b/python/tests/cli/studio/test_cp.py index e854f2aaf..0ae63fe42 100644 --- a/python/tests/cli/studio/test_cp.py +++ b/python/tests/cli/studio/test_cp.py @@ -115,3 +115,11 @@ def test_cp_download_preserves_trailing_slash_for_directory_targets(tmp_path: Pa def test_cp_studio_root_without_trailing_slash_raises(): with pytest.raises(ValueError, match="add a trailing '/'"): cp_impl(source="lit://owner/teamspace/studios/my-studio", destination="/local/out") + + +@mock_command_logging +def test_cp_help_shows_deprecation_warning(): + result_text = command_text("lightning studio cp --help") + assert "Deprecation warning:" in result_text + assert "lightning cp" in result_text + assert "Note the URL format differs" in result_text diff --git a/python/tests/cli/studio/test_ls.py b/python/tests/cli/studio/test_ls.py index 084a9098e..f02f52831 100644 --- a/python/tests/cli/studio/test_ls.py +++ b/python/tests/cli/studio/test_ls.py @@ -270,3 +270,11 @@ def test_ls_impl_nested_path(capsys): assert "january.csv" in output_lines assert "february.csv" in output_lines + + +@mock_command_logging +def test_ls_help_shows_deprecation_warning(): + result_text = command_text("lightning studio ls --help") + assert "Deprecation warning:" in result_text + assert "lightning ls" in result_text + assert "Note the URL format differs" in result_text diff --git a/python/tests/cli/studio/test_paths.py b/python/tests/cli/studio/test_paths.py new file mode 100644 index 000000000..ef47de9b8 --- /dev/null +++ b/python/tests/cli/studio/test_paths.py @@ -0,0 +1,72 @@ +import pytest + +from lightning_sdk.cli.studio.paths import parse_studio_path + + +def test_parse_studio_path_long_form(): + assert parse_studio_path("lit://org/teamspace/studios/my-studio/data/file.txt") == { + "owner": "org", + "teamspace": "teamspace", + "studio": "my-studio", + "destination": "data/file.txt", + } + + +def test_parse_studio_path_short_form_teamspace(): + result = parse_studio_path("lit://teamspace/studios/my-studio/file.txt") + assert (result["owner"], result["teamspace"], result["studio"], result["destination"]) == ( + None, + "teamspace", + "my-studio", + "file.txt", + ) + + +def test_parse_studio_path_short_form_bare_studio(): + result = parse_studio_path("lit://my-studio/file.txt") + assert (result["owner"], result["teamspace"], result["studio"], result["destination"]) == ( + None, + None, + "my-studio", + "file.txt", + ) + + +def test_parse_studio_path_relative_form(): + result = parse_studio_path("lit:///studios/my-studio/data/file.txt") + assert (result["owner"], result["teamspace"], result["studio"], result["destination"]) == ( + None, + None, + "my-studio", + "data/file.txt", + ) + + +def test_parse_studio_path_relative_form_studio_root(): + result = parse_studio_path("lit:///studios/my-studio/") + assert (result["studio"], result["destination"]) == ("my-studio", "") + + +def test_parse_studio_path_relative_form_missing_studio_raises(): + with pytest.raises(ValueError, match="Missing studio name"): + parse_studio_path("lit:///studios/") + + +def test_parse_studio_path_relative_form_requires_studios_root(): + with pytest.raises(ValueError, match="lit:///studios//"): + parse_studio_path("lit:///uploads/file.txt") + + +def test_parse_studio_path_studio_root_without_trailing_slash_raises(): + with pytest.raises(ValueError, match="add a trailing '/'"): + parse_studio_path("lit://org/teamspace/studios/my-studio") + + +def test_parse_studio_path_bare_absolute_path_raises(): + with pytest.raises(ValueError, match="Invalid studio path"): + parse_studio_path("/tmp/file") + + +def test_parse_studio_path_extra_slashes_raise(): + with pytest.raises(ValueError, match="Invalid studio path"): + parse_studio_path("lit:////studios/my-studio/file.txt") diff --git a/python/tests/cli/studio/test_rm.py b/python/tests/cli/studio/test_rm.py index 8b43557f4..1823a0e38 100644 --- a/python/tests/cli/studio/test_rm.py +++ b/python/tests/cli/studio/test_rm.py @@ -402,3 +402,11 @@ def test_rm_with_force_and_recursive(): ) # returns None without raising assert result is None + + +@mock_command_logging +def test_rm_help_shows_deprecation_warning(): + result_text = command_text("lightning studio rm --help") + assert "Deprecation warning:" in result_text + assert "lightning rm" in result_text + assert "Note the URL format differs" in result_text diff --git a/python/tests/cli/test_legacy_redirects.py b/python/tests/cli/test_legacy_redirects.py new file mode 100644 index 000000000..873e79dbe --- /dev/null +++ b/python/tests/cli/test_legacy_redirects.py @@ -0,0 +1,44 @@ +import click +from click.testing import CliRunner + +from lightning_sdk.cli.legacy_redirects import mark_deprecated_command + + +def test_mark_deprecated_command_is_idempotent() -> None: + @click.command("old") + def old_command() -> None: + click.echo("called") + + first_group = click.Group("first") + first_group.add_command(mark_deprecated_command(old_command, "lightning new")) + second_group = click.Group("second") + second_group.add_command(mark_deprecated_command(old_command, "lightning new")) + runner = CliRunner() + + for group in (first_group, second_group): + help_result = runner.invoke(group, ["old", "--help"]) + assert help_result.exit_code == 0 + assert help_result.output.count("Deprecation warning:") == 1 + + invoke_result = runner.invoke(group, ["old"]) + assert invoke_result.exit_code == 0 + assert invoke_result.output.count("Deprecation warning:") == 1 + assert "called" in invoke_result.output + + +def test_mark_deprecated_command_appends_detail() -> None: + @click.command("old") + def old_command() -> None: + click.echo("called") + + group = click.Group("g") + group.add_command(mark_deprecated_command(old_command, "lightning new", detail="Note the URL format differs.")) + runner = CliRunner() + + help_result = runner.invoke(group, ["old", "--help"]) + assert "Deprecation warning:" in help_result.output + assert "Note the URL format differs." in help_result.output + + invoke_result = runner.invoke(group, ["old"]) + assert "Note the URL format differs." in invoke_result.output + assert "called" in invoke_result.output diff --git a/python/tests/core/filesystem/test_filesystem_copy.py b/python/tests/core/filesystem/test_filesystem_copy.py index ca4c43a15..9860914e2 100644 --- a/python/tests/core/filesystem/test_filesystem_copy.py +++ b/python/tests/core/filesystem/test_filesystem_copy.py @@ -32,8 +32,8 @@ def fake_path_result(): @mock.patch("lightning_sdk.api.filesystem_api.requests.get") @mock.patch("lightning_sdk.api.utils.LightningClient") @mock.patch("lightning_sdk.api.filesystem_api._authenticate_and_get_auth_headers") -@mock.patch("lightning_sdk.filesystem.resolve_teamspace") -@mock.patch("lightning_sdk.filesystem.parse_lit_url") +@mock.patch("lightning_sdk.cli.utils.filesystem.resolve_teamspace") +@mock.patch("lightning_sdk.cli.utils.filesystem.parse_lit_url") def test_copy_download_file( mock_parse_lit_url, mock_resolve, mock_authenticate, mock_client_cls, mock_get, fake_teamspace, tmp_path ): @@ -74,8 +74,8 @@ def fake_get(url, **kwargs): @mock.patch("lightning_sdk.api.filesystem_api.requests.get") @mock.patch("lightning_sdk.api.utils.LightningClient") @mock.patch("lightning_sdk.api.filesystem_api._authenticate_and_get_auth_headers") -@mock.patch("lightning_sdk.filesystem.resolve_teamspace") -@mock.patch("lightning_sdk.filesystem.parse_lit_url") +@mock.patch("lightning_sdk.cli.utils.filesystem.resolve_teamspace") +@mock.patch("lightning_sdk.cli.utils.filesystem.parse_lit_url") def test_copy_download_folder( mock_parse_lit_url, mock_resolve, @@ -121,8 +121,8 @@ def fake_get(url, **kwargs): @mock.patch("lightning_sdk.api.filesystem_api.requests.get") @mock.patch("lightning_sdk.api.utils.LightningClient") @mock.patch("lightning_sdk.api.filesystem_api._authenticate_and_get_auth_headers", return_value=FAKE_AUTH_HEADERS) -@mock.patch("lightning_sdk.filesystem.resolve_teamspace") -@mock.patch("lightning_sdk.filesystem.parse_lit_url") +@mock.patch("lightning_sdk.cli.utils.filesystem.resolve_teamspace") +@mock.patch("lightning_sdk.cli.utils.filesystem.parse_lit_url") def test_copy_raises_if_directory_without_recursive( mock_parse_lit_url, mock_resolve, _mock_authenticate, mock_client_cls, mock_get, fake_teamspace ): @@ -144,8 +144,8 @@ def test_copy_raises_if_directory_without_recursive( @mock.patch("lightning_sdk.api.filesystem_api.requests.get") @mock.patch("lightning_sdk.api.utils.LightningClient") @mock.patch("lightning_sdk.api.filesystem_api._authenticate_and_get_auth_headers", return_value=FAKE_AUTH_HEADERS) -@mock.patch("lightning_sdk.filesystem.resolve_teamspace") -@mock.patch("lightning_sdk.filesystem.parse_lit_url") +@mock.patch("lightning_sdk.cli.utils.filesystem.resolve_teamspace") +@mock.patch("lightning_sdk.cli.utils.filesystem.parse_lit_url") def test_copy_raises_if_remote_file_not_found( mock_parse_lit_url, mock_resolve, _mock_authenticate, mock_client_cls, mock_get, fake_teamspace, fake_path_result ): @@ -178,10 +178,10 @@ def _upload_fs(fake_teamspace, destination): fs._filesystem_api = mock.Mock() patches = ( mock.patch( - "lightning_sdk.filesystem.parse_lit_url", + "lightning_sdk.cli.utils.filesystem.parse_lit_url", return_value={"teamspace": "my-teamspace", "owner": "my-org", "destination": destination}, ), - mock.patch("lightning_sdk.filesystem.resolve_teamspace", return_value=fake_teamspace), + mock.patch("lightning_sdk.cli.utils.filesystem.resolve_teamspace", return_value=fake_teamspace), ) return fs, patches diff --git a/python/tests/core/filesystem/test_filesystem_listdir.py b/python/tests/core/filesystem/test_filesystem_listdir.py index e7d1cd47a..6c3629b36 100644 --- a/python/tests/core/filesystem/test_filesystem_listdir.py +++ b/python/tests/core/filesystem/test_filesystem_listdir.py @@ -34,8 +34,8 @@ def fake_path_result(): "lightning_sdk.api.filesystem_api._authenticate_and_get_auth_headers", new=mock.MagicMock(return_value=FAKE_AUTH_HEADERS), ) -@mock.patch("lightning_sdk.filesystem.resolve_teamspace") -@mock.patch("lightning_sdk.filesystem.parse_lit_url") +@mock.patch("lightning_sdk.cli.utils.filesystem.resolve_teamspace") +@mock.patch("lightning_sdk.cli.utils.filesystem.parse_lit_url") def test_listdir_returns_files( mock_parse_lit_url, mock_resolve, mock_client_cls, mock_get, fake_teamspace, fake_path_result ): @@ -61,8 +61,8 @@ def test_listdir_returns_files( "lightning_sdk.api.filesystem_api._authenticate_and_get_auth_headers", new=mock.MagicMock(return_value=FAKE_AUTH_HEADERS), ) -@mock.patch("lightning_sdk.filesystem.resolve_teamspace") -@mock.patch("lightning_sdk.filesystem.parse_lit_url") +@mock.patch("lightning_sdk.cli.utils.filesystem.resolve_teamspace") +@mock.patch("lightning_sdk.cli.utils.filesystem.parse_lit_url") def test_listdir_passes_correct_teamspace_id(mock_parse_lit_url, mock_resolve, mock_client_cls, mock_get): mock_parse_lit_url.return_value = {"teamspace": "my-teamspace", "owner": "my-org", "destination": REMOTE_PATH} ts = mock.MagicMock() @@ -85,8 +85,8 @@ def test_listdir_passes_correct_teamspace_id(mock_parse_lit_url, mock_resolve, m "lightning_sdk.api.filesystem_api._authenticate_and_get_auth_headers", new=mock.MagicMock(return_value=FAKE_AUTH_HEADERS), ) -@mock.patch("lightning_sdk.filesystem.resolve_teamspace") -@mock.patch("lightning_sdk.filesystem.parse_lit_url") +@mock.patch("lightning_sdk.cli.utils.filesystem.resolve_teamspace") +@mock.patch("lightning_sdk.cli.utils.filesystem.parse_lit_url") def test_listdir_non_recursive( mock_parse_lit_url, mock_resolve, mock_client_cls, mock_get, fake_teamspace, fake_path_result ): diff --git a/python/tests/core/filesystem/test_filesystem_rm.py b/python/tests/core/filesystem/test_filesystem_rm.py index 46e6e91da..bf006d92f 100644 --- a/python/tests/core/filesystem/test_filesystem_rm.py +++ b/python/tests/core/filesystem/test_filesystem_rm.py @@ -21,8 +21,8 @@ def list_files(teamspace_id, path, recursive): assert path == "uploads/data" return [{"path": "test1.txt", "type": "blob", "size": 3}] - with mock.patch("lightning_sdk.filesystem.resolve_teamspace", return_value=fake_teamspace), mock.patch( - "lightning_sdk.filesystem.parse_lit_url", + with mock.patch("lightning_sdk.cli.utils.filesystem.resolve_teamspace", return_value=fake_teamspace), mock.patch( + "lightning_sdk.cli.utils.filesystem.parse_lit_url", return_value={"teamspace": "my-teamspace", "owner": "my-org", "destination": "uploads/data/test1.txt"}, ), mock.patch("lightning_sdk.filesystem.FilesystemApi") as mock_api_cls: mock_api_cls.return_value.list_files.side_effect = list_files @@ -36,8 +36,8 @@ def test_rm_directory_requires_recursive(fake_teamspace): def list_files(teamspace_id, path, recursive): return [{"path": "data", "type": "tree"}] - with mock.patch("lightning_sdk.filesystem.resolve_teamspace", return_value=fake_teamspace), mock.patch( - "lightning_sdk.filesystem.parse_lit_url", + with mock.patch("lightning_sdk.cli.utils.filesystem.resolve_teamspace", return_value=fake_teamspace), mock.patch( + "lightning_sdk.cli.utils.filesystem.parse_lit_url", return_value={"teamspace": "my-teamspace", "owner": "my-org", "destination": "uploads/data"}, ), mock.patch("lightning_sdk.filesystem.FilesystemApi") as mock_api_cls: mock_api_cls.return_value.list_files.side_effect = list_files @@ -51,8 +51,8 @@ def test_rm_directory_recursive_deletes_the_folder(fake_teamspace): def list_files(teamspace_id, path, recursive): return [{"path": "data", "type": "tree"}] - with mock.patch("lightning_sdk.filesystem.resolve_teamspace", return_value=fake_teamspace), mock.patch( - "lightning_sdk.filesystem.parse_lit_url", + with mock.patch("lightning_sdk.cli.utils.filesystem.resolve_teamspace", return_value=fake_teamspace), mock.patch( + "lightning_sdk.cli.utils.filesystem.parse_lit_url", return_value={"teamspace": "my-teamspace", "owner": "my-org", "destination": "uploads/data"}, ), mock.patch("lightning_sdk.filesystem.FilesystemApi") as mock_api_cls: mock_api_cls.return_value.list_files.side_effect = list_files @@ -63,8 +63,8 @@ def list_files(teamspace_id, path, recursive): def test_rm_missing_path_raises(fake_teamspace): - with mock.patch("lightning_sdk.filesystem.resolve_teamspace", return_value=fake_teamspace), mock.patch( - "lightning_sdk.filesystem.parse_lit_url", + with mock.patch("lightning_sdk.cli.utils.filesystem.resolve_teamspace", return_value=fake_teamspace), mock.patch( + "lightning_sdk.cli.utils.filesystem.parse_lit_url", return_value={"teamspace": "my-teamspace", "owner": "my-org", "destination": "uploads/data/missing.txt"}, ), mock.patch("lightning_sdk.filesystem.FilesystemApi") as mock_api_cls: mock_api_cls.return_value.list_files.return_value = [] @@ -73,8 +73,8 @@ def test_rm_missing_path_raises(fake_teamspace): def test_rm_refuses_teamspace_root(fake_teamspace): - with mock.patch("lightning_sdk.filesystem.resolve_teamspace", return_value=fake_teamspace), mock.patch( - "lightning_sdk.filesystem.parse_lit_url", + with mock.patch("lightning_sdk.cli.utils.filesystem.resolve_teamspace", return_value=fake_teamspace), mock.patch( + "lightning_sdk.cli.utils.filesystem.parse_lit_url", return_value={"teamspace": "my-teamspace", "owner": "my-org", "destination": ""}, ), mock.patch("lightning_sdk.filesystem.FilesystemApi"), pytest.raises(ValueError, match="teamspace root"): Filesystem().rm("lit://my-org/my-teamspace/", recursive=True) diff --git a/python/tests/core/filesystem/test_filesystem_walk.py b/python/tests/core/filesystem/test_filesystem_walk.py index 77ab3f99e..e673da4a4 100644 --- a/python/tests/core/filesystem/test_filesystem_walk.py +++ b/python/tests/core/filesystem/test_filesystem_walk.py @@ -34,8 +34,8 @@ def fake_path_result(): "lightning_sdk.api.filesystem_api._authenticate_and_get_auth_headers", new=mock.MagicMock(return_value=FAKE_AUTH_HEADERS), ) -@mock.patch("lightning_sdk.filesystem.resolve_teamspace") -@mock.patch("lightning_sdk.filesystem.parse_lit_url") +@mock.patch("lightning_sdk.cli.utils.filesystem.resolve_teamspace") +@mock.patch("lightning_sdk.cli.utils.filesystem.parse_lit_url") def test_walk_yields_os_walk_style_tuples( mock_parse_lit_url, mock_resolve, mock_client_cls, mock_get, fake_teamspace, fake_path_result ): @@ -67,8 +67,8 @@ def test_walk_yields_os_walk_style_tuples( "lightning_sdk.api.filesystem_api._authenticate_and_get_auth_headers", new=mock.MagicMock(return_value=FAKE_AUTH_HEADERS), ) -@mock.patch("lightning_sdk.filesystem.resolve_teamspace") -@mock.patch("lightning_sdk.filesystem.parse_lit_url") +@mock.patch("lightning_sdk.cli.utils.filesystem.resolve_teamspace") +@mock.patch("lightning_sdk.cli.utils.filesystem.parse_lit_url") def test_walk_flat_directory( mock_parse_lit_url, mock_resolve, mock_client_cls, mock_get, fake_teamspace, fake_path_result ): @@ -97,8 +97,8 @@ def test_walk_flat_directory( "lightning_sdk.api.filesystem_api._authenticate_and_get_auth_headers", new=mock.MagicMock(return_value=FAKE_AUTH_HEADERS), ) -@mock.patch("lightning_sdk.filesystem.resolve_teamspace") -@mock.patch("lightning_sdk.filesystem.parse_lit_url") +@mock.patch("lightning_sdk.cli.utils.filesystem.resolve_teamspace") +@mock.patch("lightning_sdk.cli.utils.filesystem.parse_lit_url") def test_walk_empty(mock_parse_lit_url, mock_resolve, mock_client_cls, mock_get, fake_teamspace, fake_path_result): mock_parse_lit_url.return_value = fake_path_result mock_resolve.return_value = fake_teamspace @@ -118,8 +118,8 @@ def test_walk_empty(mock_parse_lit_url, mock_resolve, mock_client_cls, mock_get, "lightning_sdk.api.filesystem_api._authenticate_and_get_auth_headers", new=mock.MagicMock(return_value=FAKE_AUTH_HEADERS), ) -@mock.patch("lightning_sdk.filesystem.resolve_teamspace") -@mock.patch("lightning_sdk.filesystem.parse_lit_url") +@mock.patch("lightning_sdk.cli.utils.filesystem.resolve_teamspace") +@mock.patch("lightning_sdk.cli.utils.filesystem.parse_lit_url") def test_walk_is_recursive( mock_parse_lit_url, mock_resolve, mock_client_cls, mock_get, fake_teamspace, fake_path_result ): @@ -142,8 +142,8 @@ def test_walk_is_recursive( "lightning_sdk.api.filesystem_api._authenticate_and_get_auth_headers", new=mock.MagicMock(return_value=FAKE_AUTH_HEADERS), ) -@mock.patch("lightning_sdk.filesystem.resolve_teamspace") -@mock.patch("lightning_sdk.filesystem.parse_lit_url") +@mock.patch("lightning_sdk.cli.utils.filesystem.resolve_teamspace") +@mock.patch("lightning_sdk.cli.utils.filesystem.parse_lit_url") def test_walk_passes_correct_teamspace_id(mock_parse_lit_url, mock_resolve, mock_client_cls, mock_get): mock_parse_lit_url.return_value = {"teamspace": "my-teamspace", "owner": "my-org", "destination": REMOTE_PATH} ts = mock.MagicMock() diff --git a/python/tests/core/test_request_export.py b/python/tests/core/test_request_export.py index 4b10b4a17..36857a7d9 100644 --- a/python/tests/core/test_request_export.py +++ b/python/tests/core/test_request_export.py @@ -321,6 +321,7 @@ def __call__(self): "teamspace/lightning_storage/blackbox-exports/daily/2026-04-22", "/teamspace/lightning_storage/blackbox-exports/daily/2026-04-22", "lit://my-org/my-teamspace/lightning_storage/blackbox-exports/daily/2026-04-22", + "lit:///lightning_storage/blackbox-exports/daily/2026-04-22", "lightning_storage//blackbox-exports//daily/2026-04-22", ], ) @@ -385,8 +386,9 @@ def test_export_rejects_lit_remote_path_for_other_teamspace(tmp_path, remote_pat "non-empty teamspace", ), ( + # A relative URL is valid, but its path must still be rooted at lightning_storage. "lit:///my-teamspace/lightning_storage/blackbox-exports/daily/2026-04-22", - "non-empty owner", + "lightning_storage destinations only", ), ], ) diff --git a/python/tests/utils/test_filesystem.py b/python/tests/utils/test_filesystem.py new file mode 100644 index 000000000..e6a14332e --- /dev/null +++ b/python/tests/utils/test_filesystem.py @@ -0,0 +1,61 @@ +import pytest + +from lightning_sdk.utils.filesystem import parse_lit_url + + +def test_parse_lit_url_long_form(): + assert parse_lit_url("lit://org/teamspace/uploads/data.csv") == { + "owner": "org", + "teamspace": "teamspace", + "studio": None, + "destination": "uploads/data.csv", + } + + +def test_parse_lit_url_long_form_teamspace_root(): + result = parse_lit_url("lit://org/teamspace") + assert (result["owner"], result["teamspace"], result["destination"]) == ("org", "teamspace", "") + + +def test_parse_lit_url_bare_path_without_prefix(): + result = parse_lit_url("org/teamspace/uploads/data.csv") + assert (result["owner"], result["teamspace"], result["destination"]) == ("org", "teamspace", "uploads/data.csv") + + +def test_parse_lit_url_relative_form(): + assert parse_lit_url("lit:///uploads/data.csv") == { + "owner": None, + "teamspace": None, + "studio": None, + "destination": "uploads/data.csv", + } + + +def test_parse_lit_url_relative_form_nested(): + result = parse_lit_url("lit:///studios/my-studio/notes.txt") + assert (result["owner"], result["teamspace"], result["destination"]) == (None, None, "studios/my-studio/notes.txt") + + +def test_parse_lit_url_relative_form_teamspace_root(): + result = parse_lit_url("lit:///") + assert (result["owner"], result["teamspace"], result["destination"]) == (None, None, "") + + +def test_parse_lit_url_single_segment_suggests_relative_form(): + with pytest.raises(ValueError, match="lit:///"): + parse_lit_url("lit://data.csv") + + +def test_parse_lit_url_empty_raises(): + with pytest.raises(ValueError, match="cannot be empty"): + parse_lit_url("lit://") + + +def test_parse_lit_url_bare_absolute_path_raises(): + with pytest.raises(ValueError, match="Invalid lit URL"): + parse_lit_url("/tmp/file") + + +def test_parse_lit_url_extra_slashes_raise(): + with pytest.raises(ValueError, match="Invalid lit URL"): + parse_lit_url("lit:////uploads/data.csv")