Skip to content

Remove explicit storage connection-string support in favor of workspace-linked storage - #774

Merged
v-elegacheva merged 9 commits into
microsoft:mainfrom
v-elegacheva:task/55267-deprecate-workspace-storage
Oct 13, 2026
Merged

v-elegacheva merged 9 commits into
microsoft:mainfrom
v-elegacheva:task/55267-deprecate-workspace-storage

Conversation

@v-elegacheva

@v-elegacheva v-elegacheva commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Removes explicit storage connection-string support because the Azure Quantum backend only supports storage linked to the workspace. Arbitrary storage accounts are no longer supported for Azure Quantum job storage.

Changes

  • Removed the Workspace(storage=...) constructor parameter, stored state, property, documentation, and explicit-storage container URI path.
  • Workspace.get_container_uri(...) now always uses workspace-linked storage.
  • Made parameters after name keyword-only so a legacy fourth positional argument raises TypeError instead of binding to resource_id.
  • Removed the obsolete standalone helpers:
    • azure.quantum.storage.create_container(...)
    • azure.quantum.storage.create_container_using_client(...)`
    • azure.quantum.storage.get_container_uri(...)`
  • Workspace.from_connection_string(...) is unaffected. It authenticates to the Azure Quantum workspace and it related to storage configuration.
  • Direct storage-account access remains available through azure.storage.blob.

Tests

  • Added coverage rejecting the legacy fourth positional argument and storage= keyword.
  • Added production-method coverage for linked-storage container SAS retrieval.
  • python -m pytest tests/test_workspace.py: 22 passed.
  • python -m pytest tests --ignore=tests/test_qiskit.py --ignore=tests/test_cirq.py: 138 passed.

Emit a DeprecationWarning when storage= is supplied, pointing users to configure workspace-linked storage instead. Removal is planned for azure-quantum 4.0.0. Transitional behavior (explicit-storage container URI generation) is unchanged. Workspace.from_connection_string(...) is unaffected and does not warn.
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The new tests do not independently verify the warning text or caller attribution as claimed.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Deprecates explicit workspace storage connection strings while retaining transitional behavior.

Changes:

  • Adds a DeprecationWarning for non-None storage.
  • Updates workspace storage and connection-string documentation.
  • Adds deprecation and compatibility tests.
File Description
azure-quantum/​azure/​quantum/​workspace.py Implements and documents storage deprecation.
azure-quantum/​tests/​test_workspace.py Tests warnings and retained storage behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread azure-quantum/tests/test_workspace.py Outdated
Compare against a literal string instead of Workspace._STORAGE_DEPRECATION_MESSAGE so the assertion can actually detect a regression in the warning wording (addresses PR microsoft#774 review comment).
@v-elegacheva

Copy link
Copy Markdown
Contributor Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

The stack-level test does not verify the warning’s exact attribution.

Review effort: Balanced
Findings: None

Resolved since last review (1)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Assert the exact warning caller frame to verify stacklevel=2

azure-quantum/​tests/​test_workspace.py:154

This inequality does not actually verify stacklevel=2: with WorkspaceMock, the expected warning frame is WorkspaceMock.__init__ in mock_client.py, and any other non-workspace.py frame would also pass. Assert the exact caller filename (or instantiate Workspace directly and assert this test file) so an incorrect stack level is caught.

Assert both the source file and line of WorkspaceMock.__init__'s super call so the test precisely verifies stacklevel=2 attribution.
@v-elegacheva

Copy link
Copy Markdown
Contributor Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@v-elegacheva

Copy link
Copy Markdown
Contributor Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The deprecation is correctly implemented, documented, and comprehensively tested without changing transitional behavior.

Review effort: Balanced
Findings: None

Comment thread azure-quantum/tests/test_workspace.py Outdated
@v-elegacheva v-elegacheva changed the title Deprecate Workspace(storage=...) in favor of workspace-linked storage Remove Workspace(storage=...) in favor of workspace-linked storage Oct 8, 2026
@v-elegacheva

Copy link
Copy Markdown
Contributor Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The implementation consistently removes the legacy path and provides focused regression coverage.

0 open findings

🧠 Review effort: Balanced

@v-elegacheva

Copy link
Copy Markdown
Contributor Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@v-elegacheva v-elegacheva changed the title Remove Workspace(storage=...) in favor of workspace-linked storage Remove explicit storage connection-string support in favor of workspace-linked storage Oct 9, 2026
@v-elegacheva

Copy link
Copy Markdown
Contributor Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@rigidit

Copy link
Copy Markdown
Member

update description that backend already does not support working with storages different from linked to workspace - this is the reason why we are removing that storage param

Comment thread azure-quantum/azure/quantum/workspace.py Outdated
Comment thread azure-quantum/azure/quantum/workspace.py Outdated
Comment thread azure-quantum/azure/quantum/workspace.py Outdated
Comment thread azure-quantum/azure/quantum/workspace.py Outdated
Comment thread azure-quantum/azure/quantum/workspace.py Outdated
Comment thread azure-quantum/tests/test_workspace.py Outdated
@v-elegacheva

Copy link
Copy Markdown
Contributor Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@v-elegacheva

Copy link
Copy Markdown
Contributor Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@v-elegacheva
v-elegacheva marked this pull request as ready for review October 13, 2026 20:36
@v-elegacheva
v-elegacheva requested a review from a team as a code owner October 13, 2026 20:36
@v-elegacheva
v-elegacheva merged commit 891b423 into microsoft:main Oct 13, 2026
6 checks passed
@v-elegacheva
v-elegacheva deleted the task/55267-deprecate-workspace-storage branch October 13, 2026 20:36
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.

3 participants