From dbae31177352c722cf57f43b7ac740ec8ba515ae Mon Sep 17 00:00:00 2001 From: Sarthak Joshi Date: Fri, 18 Sep 2026 16:11:04 +0100 Subject: [PATCH 1/3] fix(api): upload empty files with an explicit zero length MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit requests decides a body's length with a truth test, so a zero-byte file is indistinguishable from a body whose length it cannot determine and falls back to "Transfer-Encoding: chunked". Presigned storage PUTs reject chunked bodies with 501, which the uploader treats as transient and retries ten times with exponential backoff — minutes of apparent hang per empty file, then a failure. Send the empty body directly so requests takes its no-body path and sets "Content-Length: 0". Also extract the PUT status handling so both the empty and streaming paths share it. --- python/lightning_sdk/api/utils.py | 51 +++++++++---- python/tests/api/test_utils.py | 123 ++++++++++++++++++++++++++++++ 2 files changed, 161 insertions(+), 13 deletions(-) diff --git a/python/lightning_sdk/api/utils.py b/python/lightning_sdk/api/utils.py index 656bce7c..3d9c60fe 100644 --- a/python/lightning_sdk/api/utils.py +++ b/python/lightning_sdk/api/utils.py @@ -330,6 +330,17 @@ def _single_part_upload(self) -> None: if self.extra_headers: headers.update(self.extra_headers) + if self.filesize == 0: + # requests derives the body length with a truth test, so a zero-length + # stream is indistinguishable from one whose length it cannot know and + # falls back to "Transfer-Encoding: chunked". Presigned storage PUTs + # reject that with 501, which then trips the retry ladder below and + # costs minutes per empty file. Sending the empty body directly takes + # requests' no-body path, which sets "Content-Length: 0" instead. + r = requests.put(urls[0]["url"], data=b"", timeout=30, headers=headers or None) + self._raise_for_upload_status(r) + return + with open(self.local_path, "rb") as f: if self.show_progress: with tqdm.wrapattr( @@ -350,19 +361,33 @@ def _single_part_upload(self) -> None: else: r = requests.put(urls[0]["url"], data=f, timeout=30, headers=headers or None) - if r.status_code != 200: - # Retry transient server errors / throttling, and also 401/403 from - # storage: every attempt signs a fresh URL, which heals expired - # signatures and newly issued storage credentials that haven't - # propagated yet (e.g. right after a lightning_storage folder is - # created). The backoff decorator only retries HTTPError/ - # RequestException, so raise an HTTPError for these instead of - # failing immediately. - if r.status_code >= 500 or r.status_code in (401, 403, 429): - raise HTTPError( - f"Transient error uploading file '{self.local_path}'. Status code: {r.status_code}", response=r - ) - raise RuntimeError(f"Failed to upload file '{self.local_path}'. Status code: {r.status_code}") + self._raise_for_upload_status(r) + + def _raise_for_upload_status(self, r: requests.Response) -> None: + """Turn a non-200 storage PUT response into the right exception type. + + Args: + r: The response from the presigned storage PUT. + + Raises: + HTTPError: On statuses worth retrying, so the backoff-wrapped caller + signs a fresh URL and tries again. + RuntimeError: On any other non-200 status. + """ + if r.status_code == 200: + return + # Retry transient server errors / throttling, and also 401/403 from + # storage: every attempt signs a fresh URL, which heals expired + # signatures and newly issued storage credentials that haven't + # propagated yet (e.g. right after a lightning_storage folder is + # created). The backoff decorator only retries HTTPError/ + # RequestException, so raise an HTTPError for these instead of + # failing immediately. + if r.status_code >= 500 or r.status_code in (401, 403, 429): + raise HTTPError( + f"Transient error uploading file '{self.local_path}'. Status code: {r.status_code}", response=r + ) + raise RuntimeError(f"Failed to upload file '{self.local_path}'. Status code: {r.status_code}") def _upload_part_with_recovery(self, url_info: Dict[str, Any], upload_id: str) -> Dict[str, Any]: """Upload one part, falling back to re-signing its URL on failure. diff --git a/python/tests/api/test_utils.py b/python/tests/api/test_utils.py index 6e46624a..6ad0cc99 100644 --- a/python/tests/api/test_utils.py +++ b/python/tests/api/test_utils.py @@ -30,6 +30,8 @@ from lightning_sdk.machine import Machine _TEST_ENDPOINT_BASE = "https://api.example.com/v1/projects/test-project-id/artifacts" +# Tests that prepare a real request need a URL requests can parse, not a bare placeholder. +_TEST_SIGNED_PUT_URL = "https://storage.example.com/signed-put-url" def _make_mocked_blob_uploader(monkeypatch, file_path, remote_path, **kwargs): @@ -291,6 +293,127 @@ def test_blob_uploader_single_part_no_completion(tmp_path, monkeypatch): put_mock.assert_called_once() +def _capturing_put(captured): + """A ``requests.put`` stand-in that records the headers requests would really send. + + The other upload tests assert on the arguments handed to a ``Mock``, which never + builds a request and so cannot see headers requests derives from the body. Preparing + the request here is what makes the body-length assertions below meaningful, and is + why these tests need a fully-qualified signed URL rather than a bare placeholder. + """ + + def put(url, data=None, headers=None, **_): + prepared = requests.Request("PUT", url, data=data, headers=headers).prepare() + captured.append(prepared) + return Mock(status_code=200) + + return put + + +@pytest.mark.parametrize("progress_bar", [False, True]) +def test_blob_uploader_single_part_sends_length_for_empty_file(tmp_path, monkeypatch, progress_bar): + """An empty file uploads with an explicit zero length instead of a chunked body. + + requests falls back to chunked transfer encoding whenever it cannot determine a + body's length, and a zero-length body is indistinguishable from an unknown one by + its truth value. Presigned storage PUTs answer chunked uploads with 501, so an empty + file has to declare its length. + """ + file_path = tmp_path / "__init__.py" + file_path.write_bytes(b"") + + uploader = _make_mocked_blob_uploader(monkeypatch, file_path=str(file_path), remote_path="app/__init__.py") + uploader.show_progress = progress_bar + + captured = [] + monkeypatch.setattr( + lightning_sdk.api.utils.requests, + "post", + Mock(return_value=_blob_upload_response("app/__init__.py", "", [{"url": _TEST_SIGNED_PUT_URL}])), + ) + monkeypatch.setattr(lightning_sdk.api.utils.requests, "put", _capturing_put(captured)) + + uploader() + + assert len(captured) == 1 + assert captured[0].headers["Content-Length"] == "0" + assert "Transfer-Encoding" not in captured[0].headers + + +@pytest.mark.parametrize("progress_bar", [False, True]) +def test_blob_uploader_single_part_sends_length_for_non_empty_file(tmp_path, monkeypatch, progress_bar): + """A file with content still declares its byte count rather than streaming chunked.""" + file_path = tmp_path / "main.py" + file_path.write_bytes(b"print('hi')\n") + + uploader = _make_mocked_blob_uploader(monkeypatch, file_path=str(file_path), remote_path="app/main.py") + uploader.show_progress = progress_bar + + captured = [] + monkeypatch.setattr( + lightning_sdk.api.utils.requests, + "post", + Mock(return_value=_blob_upload_response("app/main.py", "", [{"url": _TEST_SIGNED_PUT_URL}])), + ) + monkeypatch.setattr(lightning_sdk.api.utils.requests, "put", _capturing_put(captured)) + + uploader() + + assert len(captured) == 1 + assert captured[0].headers["Content-Length"] == "12" + assert "Transfer-Encoding" not in captured[0].headers + + +def test_blob_uploader_single_part_empty_file_keeps_signed_headers(tmp_path, monkeypatch): + """The empty-file path still applies the presigned and extra headers the upload needs.""" + file_path = tmp_path / "empty.bin" + file_path.write_bytes(b"") + + uploader = _make_mocked_blob_uploader( + monkeypatch, + file_path=str(file_path), + remote_path="remote-path", + content_type="text/plain", + extra_headers={"x-ms-blob-type": "BlockBlob"}, + ) + + captured = [] + monkeypatch.setattr( + lightning_sdk.api.utils.requests, + "post", + Mock( + return_value=_blob_upload_response( + "remote-path", "", [{"url": _TEST_SIGNED_PUT_URL, "headers": {"Content-Type": "text/plain"}}] + ) + ), + ) + monkeypatch.setattr(lightning_sdk.api.utils.requests, "put", _capturing_put(captured)) + + uploader() + + assert captured[0].headers["Content-Type"] == "text/plain" + assert captured[0].headers["x-ms-blob-type"] == "BlockBlob" + assert captured[0].headers["Content-Length"] == "0" + + +def test_blob_uploader_single_part_empty_file_raises_on_client_error(tmp_path, monkeypatch): + """A non-retryable status on the empty-file path fails instead of passing silently.""" + file_path = tmp_path / "empty.bin" + file_path.write_bytes(b"") + + uploader = _make_mocked_blob_uploader(monkeypatch, file_path=str(file_path), remote_path="remote-path") + + monkeypatch.setattr( + lightning_sdk.api.utils.requests, + "post", + Mock(return_value=_blob_upload_response("remote-path", "", [{"url": _TEST_SIGNED_PUT_URL}])), + ) + monkeypatch.setattr(lightning_sdk.api.utils.requests, "put", Mock(return_value=Mock(status_code=404))) + + with pytest.raises(RuntimeError, match="404"): + uploader() + + def _make_mocked_model_uploader(monkeypatch, file_path, remote_path): # Threadpools don't like mocks as input, so we just use a regular map here monkeypatch.setattr(lightning_sdk.api.utils.ThreadPoolExecutor, "map", map) From 7874bafff5fa37bb64ba294c36c0ec559708b92e Mon Sep 17 00:00:00 2001 From: Sarthak Joshi Date: Fri, 18 Sep 2026 17:08:24 +0100 Subject: [PATCH 2/3] refactor(api): single PUT path for uploads, fail fast on 501/505 Review follow-ups on the empty-file upload fix: - Build the request body once and issue one PUT with one status check, instead of an early return with its own call site. The progress bar is now created for empty files too, which the early return had skipped. - Stop classifying 501 and 505 as transient. They reject the request itself, so retrying only burns the ten-attempt ladder before failing anyway. - Fold the new tests into one parametrized length assertion, and cover the empty body in the existing no-retry test rather than duplicating it. --- python/lightning_sdk/api/utils.py | 65 +++++++++++------------ python/tests/api/test_utils.py | 85 ++++++++----------------------- 2 files changed, 51 insertions(+), 99 deletions(-) diff --git a/python/lightning_sdk/api/utils.py b/python/lightning_sdk/api/utils.py index 3d9c60fe..8a09d6cc 100644 --- a/python/lightning_sdk/api/utils.py +++ b/python/lightning_sdk/api/utils.py @@ -71,6 +71,8 @@ def __init__(self) -> None: _MAX_WORKERS = 10 # S3, R2, and GCS all cap multipart uploads at 10,000 parts _MAX_UPLOAD_PARTS = 10000 +# 5xx statuses that reject the request itself rather than signalling a transient fault +_NON_RETRYABLE_UPLOAD_STATUSES = (501, 505) def _local_file_matches_size(local_path: str, expected_size: Optional[int]) -> bool: @@ -330,36 +332,28 @@ def _single_part_upload(self) -> None: if self.extra_headers: headers.update(self.extra_headers) - if self.filesize == 0: - # requests derives the body length with a truth test, so a zero-length - # stream is indistinguishable from one whose length it cannot know and - # falls back to "Transfer-Encoding: chunked". Presigned storage PUTs - # reject that with 501, which then trips the retry ladder below and - # costs minutes per empty file. Sending the empty body directly takes - # requests' no-body path, which sets "Content-Length: 0" instead. - r = requests.put(urls[0]["url"], data=b"", timeout=30, headers=headers or None) - self._raise_for_upload_status(r) - return - - with open(self.local_path, "rb") as f: + with contextlib.ExitStack() as stack: + handle = stack.enter_context(open(self.local_path, "rb")) + body: Any = handle if self.show_progress: - with tqdm.wrapattr( - f, - "read", - desc=f"Uploading {os.path.split(self.local_path)[1]}", - total=self.filesize, - unit="B", - unit_scale=True, - unit_divisor=1024, - ) as wrapped_file: - r = requests.put( - urls[0]["url"], - data=_IterableFileWrapper(wrapped_file), - timeout=30, - headers=headers or None, + body = _IterableFileWrapper( + stack.enter_context( + tqdm.wrapattr( + handle, + "read", + desc=f"Uploading {os.path.split(self.local_path)[1]}", + total=self.filesize, + unit="B", + unit_scale=True, + unit_divisor=1024, + ) ) - else: - r = requests.put(urls[0]["url"], data=f, timeout=30, headers=headers or None) + ) + if self.filesize == 0: + # requests infers length by truth test, so an empty stream would go out + # "Transfer-Encoding: chunked", which presigned PUTs reject with 501. + body = b"" + r = requests.put(urls[0]["url"], data=body, timeout=30, headers=headers or None) self._raise_for_upload_status(r) @@ -376,14 +370,13 @@ def _raise_for_upload_status(self, r: requests.Response) -> None: """ if r.status_code == 200: return - # Retry transient server errors / throttling, and also 401/403 from - # storage: every attempt signs a fresh URL, which heals expired - # signatures and newly issued storage credentials that haven't - # propagated yet (e.g. right after a lightning_storage folder is - # created). The backoff decorator only retries HTTPError/ - # RequestException, so raise an HTTPError for these instead of - # failing immediately. - if r.status_code >= 500 or r.status_code in (401, 403, 429): + # 401/403 are worth retrying here because each attempt signs a fresh URL, which + # heals expired signatures and storage credentials that haven't propagated yet. + # Backoff only retries HTTPError/RequestException, hence the type split. + retryable = r.status_code not in _NON_RETRYABLE_UPLOAD_STATUSES and ( + r.status_code >= 500 or r.status_code in (401, 403, 429) + ) + if retryable: raise HTTPError( f"Transient error uploading file '{self.local_path}'. Status code: {r.status_code}", response=r ) diff --git a/python/tests/api/test_utils.py b/python/tests/api/test_utils.py index 6ad0cc99..cc085711 100644 --- a/python/tests/api/test_utils.py +++ b/python/tests/api/test_utils.py @@ -296,31 +296,29 @@ def test_blob_uploader_single_part_no_completion(tmp_path, monkeypatch): def _capturing_put(captured): """A ``requests.put`` stand-in that records the headers requests would really send. - The other upload tests assert on the arguments handed to a ``Mock``, which never - builds a request and so cannot see headers requests derives from the body. Preparing - the request here is what makes the body-length assertions below meaningful, and is - why these tests need a fully-qualified signed URL rather than a bare placeholder. + The other upload tests assert on the arguments handed to a ``Mock``, which never builds + a request and so cannot see headers requests derives from the body. Preparing it here is + what makes the length assertions meaningful, and why these tests need a parseable URL. """ def put(url, data=None, headers=None, **_): - prepared = requests.Request("PUT", url, data=data, headers=headers).prepare() - captured.append(prepared) + captured.append(requests.Request("PUT", url, data=data, headers=headers).prepare()) return Mock(status_code=200) return put @pytest.mark.parametrize("progress_bar", [False, True]) -def test_blob_uploader_single_part_sends_length_for_empty_file(tmp_path, monkeypatch, progress_bar): - """An empty file uploads with an explicit zero length instead of a chunked body. +@pytest.mark.parametrize(("content", "expected_length"), [(b"", "0"), (b"print('hi')\n", "12")]) +def test_blob_uploader_single_part_declares_body_length(tmp_path, monkeypatch, progress_bar, content, expected_length): + """Single-part uploads declare their length instead of streaming chunked. - requests falls back to chunked transfer encoding whenever it cannot determine a - body's length, and a zero-length body is indistinguishable from an unknown one by - its truth value. Presigned storage PUTs answer chunked uploads with 501, so an empty - file has to declare its length. + requests falls back to chunked transfer encoding when it cannot determine a body's + length, and a zero-length body is indistinguishable from an unknown one by its truth + value. Presigned storage PUTs answer chunked uploads with 501. """ file_path = tmp_path / "__init__.py" - file_path.write_bytes(b"") + file_path.write_bytes(content) uploader = _make_mocked_blob_uploader(monkeypatch, file_path=str(file_path), remote_path="app/__init__.py") uploader.show_progress = progress_bar @@ -336,36 +334,12 @@ def test_blob_uploader_single_part_sends_length_for_empty_file(tmp_path, monkeyp uploader() assert len(captured) == 1 - assert captured[0].headers["Content-Length"] == "0" - assert "Transfer-Encoding" not in captured[0].headers - - -@pytest.mark.parametrize("progress_bar", [False, True]) -def test_blob_uploader_single_part_sends_length_for_non_empty_file(tmp_path, monkeypatch, progress_bar): - """A file with content still declares its byte count rather than streaming chunked.""" - file_path = tmp_path / "main.py" - file_path.write_bytes(b"print('hi')\n") - - uploader = _make_mocked_blob_uploader(monkeypatch, file_path=str(file_path), remote_path="app/main.py") - uploader.show_progress = progress_bar - - captured = [] - monkeypatch.setattr( - lightning_sdk.api.utils.requests, - "post", - Mock(return_value=_blob_upload_response("app/main.py", "", [{"url": _TEST_SIGNED_PUT_URL}])), - ) - monkeypatch.setattr(lightning_sdk.api.utils.requests, "put", _capturing_put(captured)) - - uploader() - - assert len(captured) == 1 - assert captured[0].headers["Content-Length"] == "12" + assert captured[0].headers["Content-Length"] == expected_length assert "Transfer-Encoding" not in captured[0].headers def test_blob_uploader_single_part_empty_file_keeps_signed_headers(tmp_path, monkeypatch): - """The empty-file path still applies the presigned and extra headers the upload needs.""" + """The empty body still carries the presigned and extra headers the upload needs.""" file_path = tmp_path / "empty.bin" file_path.write_bytes(b"") @@ -396,24 +370,6 @@ def test_blob_uploader_single_part_empty_file_keeps_signed_headers(tmp_path, mon assert captured[0].headers["Content-Length"] == "0" -def test_blob_uploader_single_part_empty_file_raises_on_client_error(tmp_path, monkeypatch): - """A non-retryable status on the empty-file path fails instead of passing silently.""" - file_path = tmp_path / "empty.bin" - file_path.write_bytes(b"") - - uploader = _make_mocked_blob_uploader(monkeypatch, file_path=str(file_path), remote_path="remote-path") - - monkeypatch.setattr( - lightning_sdk.api.utils.requests, - "post", - Mock(return_value=_blob_upload_response("remote-path", "", [{"url": _TEST_SIGNED_PUT_URL}])), - ) - monkeypatch.setattr(lightning_sdk.api.utils.requests, "put", Mock(return_value=Mock(status_code=404))) - - with pytest.raises(RuntimeError, match="404"): - uploader() - - def _make_mocked_model_uploader(monkeypatch, file_path, remote_path): # Threadpools don't like mocks as input, so we just use a regular map here monkeypatch.setattr(lightning_sdk.api.utils.ThreadPoolExecutor, "map", map) @@ -834,9 +790,9 @@ def test_resolve_path_mappings(): assert path_mappings[1].connection_path == "" -def _make_single_part_blob_uploader(tmp_path, progress_bar=False, extra_headers=None): +def _make_single_part_blob_uploader(tmp_path, progress_bar=False, extra_headers=None, content=b"hello world"): file_path = tmp_path / "test_file.txt" - file_path.write_text("hello world") + file_path.write_bytes(content) with mock.patch( "lightning_sdk.api.utils._authenticate_and_get_auth_headers", return_value={"Authorization": "Bearer test-token"}, @@ -949,14 +905,17 @@ def test_single_part_uploader_retries_auth_errors_with_fresh_url(mock_requests, assert mock_requests.post.call_count == 2 +# 404 rejects the request, 501 rejects the protocol: neither improves on a retry, and the +# empty-body path has to reach the same verdict as the streaming one. +@pytest.mark.parametrize("status_code", [404, 501]) +@pytest.mark.parametrize("content", [b"hello world", b""]) @mock.patch("time.sleep", return_value=None) @mock.patch("lightning_sdk.api.utils.requests") -def test_single_part_uploader_does_not_retry_client_error(mock_requests, _, tmp_path): - uploader = _make_single_part_blob_uploader(tmp_path) +def test_single_part_uploader_does_not_retry_client_error(mock_requests, _, tmp_path, content, status_code): + uploader = _make_single_part_blob_uploader(tmp_path, content=content) _configure_single_part_create(mock_requests) - # A non-transient 4xx should fail immediately without retrying. - mock_requests.put.return_value = Mock(status_code=404) + mock_requests.put.return_value = Mock(status_code=status_code) with pytest.raises(RuntimeError, match="Failed to upload file"): uploader() From 9d2be9c94f9e14b655f284dbc7bf24af6f537aba Mon Sep 17 00:00:00 2001 From: Sarthak Joshi Date: Mon, 21 Sep 2026 11:33:08 +0100 Subject: [PATCH 3/3] test(cli): cover that command errors reach the error panel The CLI turns an unhandled error into a titled panel with a LIGHTNING_DEBUG hint, but only because the error travels out of Click's main() to the interpreter's excepthook. Existing tests call the formatter and the hook directly, so nothing covered that last link, and removing the hook's installation left the suite green while every command started printing raw tracebacks. Add coverage for the link itself: that running a command installs the hook, that Click lets a plain error escape, and end to end in a real interpreter that a failing command prints the panel rather than a traceback - and still prints the traceback under LIGHTNING_DEBUG=1. The end-to-end driver runs from a temp directory, so it pins PYTHONPATH to the package pytest imported; otherwise an installed copy shadows the checkout and the test silently passes against the wrong code. --- python/tests/cli/utils/test_logging.py | 99 ++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/python/tests/cli/utils/test_logging.py b/python/tests/cli/utils/test_logging.py index 72eebb82..446de4e4 100644 --- a/python/tests/cli/utils/test_logging.py +++ b/python/tests/cli/utils/test_logging.py @@ -9,12 +9,17 @@ """ import contextlib +import os +import pathlib +import subprocess import sys from unittest import mock import click import click.testing +import pytest +import lightning_sdk from lightning_sdk.__version__ import __version__ from lightning_sdk.cli.utils.logging import ( CommandLoggingGroup, @@ -471,3 +476,97 @@ def subcommand(): assert result.exit_code == 0 assert "Hello" in result.output + + +# The tests above prove _notify_exception renders a clean panel and that logging_excepthook calls +# it. Neither says the hook is ever reached: Click formats ClickException itself and lets every +# other error escape, so the panel depends on an ordinary error travelling all the way out of +# main() to the interpreter. These cover that last link, which is invisible to CliRunner because +# the runner catches exceptions before the interpreter ever sees them. + +# Rejected before any network call, so it exercises the failure path offline. +_FAILING_COMMAND = ["cp", "lit:///a/b", "lit:///c/d"] + + +class TestErrorsReachTheExcepthook: + """A plain error from a command has to reach sys.excepthook for the CLI panel to appear.""" + + def test_running_a_command_installs_the_excepthook(self, monkeypatch): + """The group callback points sys.excepthook at the handler that prints the panel.""" + from lightning_sdk.cli.entrypoint import main_cli + + monkeypatch.setattr(sys, "excepthook", sys.__excepthook__) + monkeypatch.setattr("lightning_sdk.cli.utils.logging._log_command", mock.Mock()) + + click.testing.CliRunner().invoke(main_cli, _FAILING_COMMAND, prog_name="lightning") + + assert sys.excepthook is logging_excepthook + + def test_click_lets_a_plain_error_escape(self, monkeypatch): + """Click formats only ClickException, so an SDK error must come out of main() unchanged.""" + from lightning_sdk.cli.entrypoint import main_cli + + monkeypatch.setattr(sys, "excepthook", sys.__excepthook__) + monkeypatch.setattr("lightning_sdk.cli.utils.logging._log_command", mock.Mock()) + + with pytest.raises(ValueError, match="Cannot copy between two remote URLs"): + main_cli.main(_FAILING_COMMAND, prog_name="lightning", standalone_mode=True) + + +# Driving a real interpreter is the only way to see what a user sees: the hook runs after the +# exception has left every frame a test could wrap. _log_command is started rather than scoped +# for the same reason - a with block would unwind before the hook fires, letting it hit the network. +# The driver lives in a temp dir, so sys.path[0] is that dir and an installed copy of the package +# would win over this checkout; _PACKAGE_ROOT pins the subprocess to the same one pytest imported. +_CLI_FAILURE_DRIVER = """ +import sys +from unittest import mock + +mock.patch("lightning_sdk.cli.utils.logging._log_command").start() +from lightning_sdk.cli.entrypoint import main_cli + +sys.argv = ["lightning", *{args!r}] +main_cli() +""" + + +_PACKAGE_ROOT = str(pathlib.Path(lightning_sdk.__file__).resolve().parents[1]) + + +def _run_cli_driver(tmp_path, debug): + driver = tmp_path / "driver.py" + driver.write_text(_CLI_FAILURE_DRIVER.format(args=_FAILING_COMMAND)) + result = subprocess.run( + [sys.executable, str(driver)], + capture_output=True, + text=True, + env={ + **os.environ, + "PYTHONPATH": _PACKAGE_ROOT, + "LIGHTNING_API_KEY": "dummy-api-key", + "COLUMNS": "200", + "LIGHTNING_DEBUG": debug, + }, + check=False, + ) + return result.returncode, result.stdout + result.stderr + + +def test_cli_reports_errors_as_a_panel_not_a_traceback(tmp_path): + """End to end: a failing command prints the error panel and no traceback.""" + returncode, output = _run_cli_driver(tmp_path, debug="") + + assert returncode == 1 + assert "Lightning CLI Error" in output + assert "Cannot copy between two remote URLs" in output + assert "Traceback (most recent call last)" not in output + assert "LIGHTNING_DEBUG=1" in output + + +def test_cli_shows_the_traceback_when_debug_is_set(tmp_path): + """LIGHTNING_DEBUG=1 is the documented way back to the traceback, so it has to work.""" + returncode, output = _run_cli_driver(tmp_path, debug="1") + + assert returncode == 1 + assert "Full traceback" in output + assert "Cannot copy between two remote URLs" in output