Skip to content

Report missing cloud SDK dependencies before client setup - #601

Open
terminalchai wants to merge 2 commits into
drivendataorg:masterfrom
terminalchai:fix/check-sdk-dependencies-before-client-init
Open

terminalchai wants to merge 2 commits into
drivendataorg:masterfrom
terminalchai:fix/check-sdk-dependencies-before-client-init

Conversation

@terminalchai

@terminalchai terminalchai commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Constructing an S3 or GCS client without its optional SDK can raise NameError before the existing dependency validation runs. Validate dependencies at the start of those constructors so direct clients, default clients and paths report MissingDependenciesError with the appropriate install command. Azure already validates before using SDK names.

Closes #441.

The regression tests cover three providers and five construction entry points. They reuse run_isolated from tests/utils.py, extracted from the existing GCS import-fallback tests. The helper starts a fresh interpreter and blocks modules through sys.modules; child scripts print error messages, and pytest checks the exception type, client name and install hint in the parent process.

Validation on Windows with Python 3.11:

  • All 27 missing-dependency and GCS-specific tests pass, including the existing optional-module fallbacks.
  • Full mocked/local suite: 2,180 passed, 127 skipped across all provider rigs.
  • The original fix reproduced 6 failures and 9 passing compatibility cases before the source change; all 15 construction cases pass after it.
  • Source and wheel distributions were built for the source fix. Installing that wheel without extras or dependencies in a fresh virtual environment also passed all 15 construction checks with the SDKs actually absent.
  • Repository-wide Black and flake8 checks, mypy on 31 source files, and git diff --check pass.
  • Regenerated changelog docs and built with MkDocs; existing link warnings remain.
  • Codex reviewed the current test refactor and found no actionable defects. Review used supplied text because its nested shell runner was unavailable; verification commands ran separately.
  • Live cloud backends were not tested locally.

Contributor checklist:

  • Read and understood CONTRIBUTING.md
  • Existing issue and Closes Missing dependency message does not appear when it should #441 in summary
  • Based on latest master
  • Failure-before / success-after evidence
  • Generic behavior addressed for both affected providers
  • Manual live cloud backend testing
  • Added tests
  • Lint passes
  • Tests pass
  • Updated HISTORY.md under UNRELEASED with issue and PR references

Prepared with AI assistance; the diff and test results were reviewed before submission.

Comment thread tests/test_missing_dependencies.py Outdated
raise ModuleNotFoundError(f"No module named {name!r}")
return original_import(name, globals, locals, fromlist, level)

builtins.__import__ = import_without_sdk

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_run_isolated in tests/test_gs_specific.py already starts a fresh interpreter and makes modules unimportable with sys.modules[name] = None.

Let's add it to utils build a way to reuse that path instead across tests and providers.

Comment thread tests/test_missing_dependencies.py Outdated
Comment on lines +58 to +62
except MissingDependenciesError as error:
assert client_name in str(error)
assert f"pip install cloudpathlib[{provider}]" in str(error)
else:
raise AssertionError("Expected MissingDependenciesError")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Print messages and assert correct output from the subprocess in the main process rather than asserting so the subprocess fails

@pjbull pjbull left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(wrong thread, moved)

This branch is waiting to be deployed

1 waiting deployment
reviewed-live-cloud — 141205a8 Waiting Oct 13, 2026 by terminalchai via Live Tests (azure_gen2) #35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing dependency message does not appear when it should

2 participants