From 506091468c5197132c54efd4adb0b85a32ec6e82 Mon Sep 17 00:00:00 2001 From: terminalchai <213856599+terminalchai@users.noreply.github.com> Date: Sun, 11 Oct 2026 01:05:17 +0530 Subject: [PATCH 1/2] fix: validate cloud SDK dependencies before client setup --- HISTORY.md | 2 + cloudpathlib/gs/gsclient.py | 2 + cloudpathlib/s3/s3client.py | 2 + tests/test_missing_dependencies.py | 71 ++++++++++++++++++++++++++++++ 4 files changed, 77 insertions(+) create mode 100644 tests/test_missing_dependencies.py diff --git a/HISTORY.md b/HISTORY.md index 1ae9db02..a0914f2a 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -8,6 +8,8 @@ - Fixed mypy missing-import errors for `CloudPath` and `AnyPath` when the optional Pydantic integration is not installed. (Issue [#385](https://github.com/drivendataorg/cloudpathlib/issues/385), PR [#603](https://github.com/drivendataorg/cloudpathlib/pull/603)) +- Fixed direct S3 and GCS client construction without the optional cloud SDK to raise `MissingDependenciesError` with the appropriate install command instead of `NameError`. (Issue [#441](https://github.com/drivendataorg/cloudpathlib/issues/441)) + ## v0.26.0 (2026-10-01) - Added support for Python 3.15: `CloudPath.mkdir(parent_mode=...)` works across providers, and cloud paths can be pickled repeatedly without a `ParseResult` error. (Issue [#593](https://github.com/drivendataorg/cloudpathlib/issues/593), PR [#594](https://github.com/drivendataorg/cloudpathlib/pull/594)) diff --git a/cloudpathlib/gs/gsclient.py b/cloudpathlib/gs/gsclient.py index 8f967edb..782f8596 100644 --- a/cloudpathlib/gs/gsclient.py +++ b/cloudpathlib/gs/gsclient.py @@ -156,6 +156,8 @@ def __init__( Defaults to the `CLOUDPATHLIB_STREAMING_MAX_CONCURRENCY` environment variable or 4; 1 makes each stream fully sequential. """ + self._cloud_meta.validate_completeness() + # don't check `GOOGLE_APPLICATION_CREDENTIALS` since `google_default_auth` already does that # use explicit client if storage_client is not None: diff --git a/cloudpathlib/s3/s3client.py b/cloudpathlib/s3/s3client.py index 651b154f..3e1e0a99 100644 --- a/cloudpathlib/s3/s3client.py +++ b/cloudpathlib/s3/s3client.py @@ -94,6 +94,8 @@ def __init__( Defaults to the `CLOUDPATHLIB_STREAMING_MAX_CONCURRENCY` environment variable or 4; 1 makes each stream fully sequential. """ + self._cloud_meta.validate_completeness() + endpoint_url = endpoint_url or os.getenv("AWS_ENDPOINT_URL") if boto3_session is not None: self.sess = boto3_session diff --git a/tests/test_missing_dependencies.py b/tests/test_missing_dependencies.py new file mode 100644 index 00000000..a5b82c99 --- /dev/null +++ b/tests/test_missing_dependencies.py @@ -0,0 +1,71 @@ +import subprocess +import sys +import textwrap + +import pytest + + +@pytest.mark.parametrize("provider", ["s3", "gs", "azure"]) +@pytest.mark.parametrize( + "constructor", ["client", "client_with_options", "default_client", "path", "dispatch"] +) +def test_missing_sdk_reports_install_hint(provider, constructor): + # Use a fresh interpreter so missing imports cannot be masked by loaded SDK modules. + script = textwrap.dedent(""" + import builtins + import sys + + provider, constructor = sys.argv[1:] + blocked_roots = { + "s3": {"boto3", "botocore"}, + "gs": {"google"}, + "azure": {"azure"}, + }[provider] + original_import = builtins.__import__ + + def import_without_sdk(name, globals=None, locals=None, fromlist=(), level=0): + if level == 0 and name.split(".")[0] in blocked_roots: + raise ModuleNotFoundError(f"No module named {name!r}") + return original_import(name, globals, locals, fromlist, level) + + builtins.__import__ = import_without_sdk + import cloudpathlib + from cloudpathlib.exceptions import MissingDependenciesError + + client_name, path_name, cloud_path, options = { + "s3": ("S3Client", "S3Path", "s3://bucket/file", {"no_sign_request": True}), + "gs": ( + "GSClient", "GSPath", "gs://bucket/file", + {"application_credentials": "missing-credentials.json"}, + ), + "azure": ( + "AzureBlobClient", "AzureBlobPath", "az://container/file", + {"connection_string": "AccountName=fake;AccountKey=fake;"}, + ), + }[provider] + client_class = getattr(cloudpathlib, client_name) + try: + if constructor == "client": + client_class() + elif constructor == "client_with_options": + client_class(**options) + elif constructor == "default_client": + client_class.get_default_client() + elif constructor == "path": + getattr(cloudpathlib, path_name)(cloud_path) + else: + cloudpathlib.CloudPath(cloud_path) + except MissingDependenciesError as error: + assert client_name in str(error) + assert f"pip install cloudpathlib[{provider}]" in str(error) + else: + raise AssertionError("Expected MissingDependenciesError") + """) + result = subprocess.run( + [sys.executable, "-c", script, provider, constructor], + capture_output=True, + text=True, + timeout=30, + ) + + assert result.returncode == 0, result.stdout + result.stderr From d6d7e15c1113b886469c3f17e5c1276e9b9e5d0a Mon Sep 17 00:00:00 2001 From: terminalchai <213856599+terminalchai@users.noreply.github.com> Date: Wed, 14 Oct 2026 03:01:52 +0530 Subject: [PATCH 2/2] test: share isolated dependency checks across providers --- HISTORY.md | 2 +- tests/test_gs_specific.py | 30 +++++------------ tests/test_missing_dependencies.py | 54 ++++++++++-------------------- tests/utils.py | 20 +++++++++++ 4 files changed, 48 insertions(+), 58 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index a0914f2a..d65f243d 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -8,7 +8,7 @@ - Fixed mypy missing-import errors for `CloudPath` and `AnyPath` when the optional Pydantic integration is not installed. (Issue [#385](https://github.com/drivendataorg/cloudpathlib/issues/385), PR [#603](https://github.com/drivendataorg/cloudpathlib/pull/603)) -- Fixed direct S3 and GCS client construction without the optional cloud SDK to raise `MissingDependenciesError` with the appropriate install command instead of `NameError`. (Issue [#441](https://github.com/drivendataorg/cloudpathlib/issues/441)) +- Fixed direct S3 and GCS client construction without the optional cloud SDK to raise `MissingDependenciesError` with the appropriate install command instead of `NameError`. (Issue [#441](https://github.com/drivendataorg/cloudpathlib/issues/441), PR [#601](https://github.com/drivendataorg/cloudpathlib/pull/601)) ## v0.26.0 (2026-10-01) diff --git a/tests/test_gs_specific.py b/tests/test_gs_specific.py index 46673636..a0fffbd1 100644 --- a/tests/test_gs_specific.py +++ b/tests/test_gs_specific.py @@ -1,5 +1,3 @@ -import subprocess -import sys from urllib.parse import urlparse, parse_qs from google.api_core import retry @@ -8,6 +6,7 @@ from cloudpathlib import GSPath from cloudpathlib.local import LocalGSPath +from tests.utils import run_isolated @pytest.mark.parametrize("path_class", [GSPath, LocalGSPath]) @@ -119,26 +118,12 @@ def test_timeout_and_retry(gs_rig): # class with the implementation registry and desynchronise every other test. -def _run_isolated(blocked_modules, body): - """Import cloudpathlib.gs.gsclient with `blocked_modules` made unimportable, run `body`, - and return its stdout.""" - code = "\n".join( - ["import sys"] - + [f"sys.modules[{name!r}] = None" for name in blocked_modules] - + ["import cloudpathlib.gs.gsclient as gsclient", body] - ) - result = subprocess.run( - [sys.executable, "-c", code], capture_output=True, text=True, timeout=120 - ) - assert result.returncode == 0, result.stderr - return result.stdout.strip() - - def test_import_without_google_cloud_storage(): """With the GCS SDK missing, the module imports, marks the dependency as unavailable, and leaves the optional transfer manager unset.""" - out = _run_isolated( + out = run_isolated( ["google.cloud.storage.client"], + "import cloudpathlib.gs.gsclient as gsclient\n" 'print(gsclient.implementation_registry["gs"].dependencies_loaded, ' "gsclient.transfer_manager is None)", ) @@ -148,8 +133,9 @@ def test_import_without_google_cloud_storage(): def test_import_xml_multipart_from_resumable_media_fallback(): """Older google-cloud-storage versions ship the XML multipart classes in google-resumable-media instead of the storage package.""" - out = _run_isolated( + out = run_isolated( ["google.cloud.storage._media.requests"], + "import cloudpathlib.gs.gsclient as gsclient\n" "import google.resumable_media.requests as fallback\n" "print(gsclient.XMLMPUContainer is fallback.XMLMPUContainer)", ) @@ -159,16 +145,18 @@ def test_import_xml_multipart_from_resumable_media_fallback(): def test_streaming_write_requires_xml_multipart_support(): """Without any XML multipart implementation, reads still work but a streaming write fails with an actionable error.""" - out = _run_isolated( + out = run_isolated( ["google.cloud.storage._media.requests", "google.resumable_media.requests"], + "import cloudpathlib.gs.gsclient as gsclient\n" "from cloudpathlib.exceptions import CloudPathNotImplementedError\n" - "assert gsclient.XMLMPUContainer is None and gsclient.XMLMPUPart is None\n" + "print(gsclient.XMLMPUContainer is None, gsclient.XMLMPUPart is None)\n" "client = gsclient.GSClient.__new__(gsclient.GSClient) # no credentials needed\n" "try:\n" " gsclient.GSClient._initiate_multipart_upload(client, None)\n" "except CloudPathNotImplementedError as e:\n" " print(e)", ) + assert out.splitlines()[0] == "True True" assert "google-cloud-storage 2.10 or newer" in out diff --git a/tests/test_missing_dependencies.py b/tests/test_missing_dependencies.py index a5b82c99..ee3d1863 100644 --- a/tests/test_missing_dependencies.py +++ b/tests/test_missing_dependencies.py @@ -1,37 +1,24 @@ -import subprocess -import sys -import textwrap - import pytest +from tests.utils import run_isolated + @pytest.mark.parametrize("provider", ["s3", "gs", "azure"]) @pytest.mark.parametrize( "constructor", ["client", "client_with_options", "default_client", "path", "dispatch"] ) def test_missing_sdk_reports_install_hint(provider, constructor): - # Use a fresh interpreter so missing imports cannot be masked by loaded SDK modules. - script = textwrap.dedent(""" - import builtins - import sys - - provider, constructor = sys.argv[1:] - blocked_roots = { - "s3": {"boto3", "botocore"}, - "gs": {"google"}, - "azure": {"azure"}, - }[provider] - original_import = builtins.__import__ - - def import_without_sdk(name, globals=None, locals=None, fromlist=(), level=0): - if level == 0 and name.split(".")[0] in blocked_roots: - raise ModuleNotFoundError(f"No module named {name!r}") - return original_import(name, globals, locals, fromlist, level) - - builtins.__import__ = import_without_sdk + blocked_modules = { + "s3": ["boto3", "botocore"], + "gs": ["google"], + "azure": ["azure"], + }[provider] + client_name = {"s3": "S3Client", "gs": "GSClient", "azure": "AzureBlobClient"}[provider] + script = """ import cloudpathlib from cloudpathlib.exceptions import MissingDependenciesError + provider, constructor = sys.argv[1:] client_name, path_name, cloud_path, options = { "s3": ("S3Client", "S3Path", "s3://bucket/file", {"no_sign_request": True}), "gs": ( @@ -56,16 +43,11 @@ def import_without_sdk(name, globals=None, locals=None, fromlist=(), level=0): else: cloudpathlib.CloudPath(cloud_path) except MissingDependenciesError as error: - assert client_name in str(error) - assert f"pip install cloudpathlib[{provider}]" in str(error) - else: - raise AssertionError("Expected MissingDependenciesError") - """) - result = subprocess.run( - [sys.executable, "-c", script, provider, constructor], - capture_output=True, - text=True, - timeout=30, - ) - - assert result.returncode == 0, result.stdout + result.stderr + print(type(error).__name__) + print(error) + """ + out = run_isolated(blocked_modules, script, args=(provider, constructor)) + + assert out.splitlines()[0:1] == ["MissingDependenciesError"] + assert client_name in out + assert f"pip install cloudpathlib[{provider}]" in out diff --git a/tests/utils.py b/tests/utils.py index 1f8c0c2c..97fe3e39 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -2,11 +2,31 @@ import os from pathlib import Path import platform +import subprocess +import sys +import textwrap import time from typing import Any, Callable, Iterable, List, Sequence, Tuple import weakref +def run_isolated(blocked_modules, body, args=()): + """Run `body` in a fresh interpreter with `blocked_modules` made unimportable. + + Return stdout so the calling test can assert on the subprocess's output. + """ + code = "\n".join( + ["import sys"] + + [f"sys.modules[{name!r}] = None" for name in blocked_modules] + + [textwrap.dedent(body)] + ) + result = subprocess.run( + [sys.executable, "-c", code, *args], capture_output=True, text=True, timeout=120 + ) + assert result.returncode == 0, result.stdout + result.stderr + return result.stdout.strip() + + def getenv(name: str, default=None): """Treat an empty environment value as unset (as in per-provider CI jobs).""" return os.getenv(name) or default