[Storage] Use ArmCustomPatch template by pshao25 · Pull Request #43207 · Azure/azure-rest-api-specs

pshao25 · 2026-05-14T10:54:23Z

Choose a PR Template

Switch to "Preview" on this description then select one of the choices below.

Click here to open a PR for a Data Plane API.

Click here to open a PR for a Control Plane (ARM) API.

Click here to open a PR for only SDK configuration.

github-actions · 2026-05-14T10:55:20Z

Next Steps to Merge

✅ All automated merging requirements have been met! To get your PR merged, see aka.ms/azsdk/specreview/merge.

Comment generated by summarize-checks workflow run.

github-actions · 2026-05-14T11:03:44Z

API Change Check

APIView identified API level changes in this PR and created the following API reviews

Language	API Review for Package
Swagger	Microsoft.Storage
TypeSpec	Microsoft.Storage
Go	sdk/resourcemanager/storage/armstorage
JavaScript	@azure/arm-storage
Python	azure-mgmt-storage
Java	com.azure.resourcemanager:azure-resourcemanager-storage
C#	Azure.ResourceManager.Storage

Comment generated by After APIView workflow run.

ravimeda · 2026-05-15T15:21:55Z

[NEW] 🟡 Warning [Uniform versioning — stable API version modified; Go SDK breaking change unresolved]

This PR modifies two already-shipped stable API versions on main:

specification/storage/resource-manager/Microsoft.Storage/stable/2025-08-01/openapi.json (merged in PR New Api-version 2025-08-01 for Microsoft.Storage #39307)
specification/storage/resource-manager/Microsoft.Storage/stable/2026-04-01/openapi.json (merged in PR Add API version 2026-04-01 for Microsoft.Storage #41271)

Per the uniform-versioning policy, shipped stable API versions are immutable. The change is a definition rename (Azure.ResourceManager.CommonTypes.TrackedResourceUpdate → TrackedResourceUpdate) which is shape-preserving at the wire level, but it causes SDK definition/class-name churn.

The PR carries BreakingChange-JavaScript-Sdk-Approved and BreakingChange-Python-Sdk-Approved, but BreakingChange-Go-Sdk is present with no -Approved companion. The PR MUST NOT merge until the Go SDK breaking-change approval is obtained or the Go SDK owners have signed off that the rename is a no-op for the generated Go client.

Classification: [NEW] — introduced in this PR.

Fix: Obtain BreakingChange-Go-Sdk-Approved before merging. Alternatively, if the rename is truly a no-op for the Go SDK (the generated struct name is derived from the wire body, not the JSON definition name), document this explicitly so reviewers can verify.

ravimeda · 2026-05-15T15:21:56Z

 };

+/**
+ * A Connector is a tracked ARM resource modeled as a sub-resource of a Storage Account.


[NEW] 🟡 Warning [TSP — Misleading docstring on PATCH request body model]

The docstring says:```typespec
/**

A Connector is a tracked ARM resource modeled as a sub-resource of a Storage Account.
*/
model ConnectorUpdate extends TrackedResourceUpdate { ... }```But ConnectorUpdate is the PATCH request body — not a tracked ARM resource. This wording was copy-pasted from the `Connector` tracked resource model. The incorrect description leaks into the generated SDK XML/JSDoc/PyDoc as misleading API documentation.

Classification: [NEW] — ConnectorUpdate was added in this PR.

Fix:```diff
-/**

A Connector is a tracked ARM resource modeled as a sub-resource of a Storage Account.

/
+/*

The updatable properties of a Storage Connector. Used as the body of the Update (PATCH) operation.

*/```

ravimeda · 2026-05-15T15:21:57Z

 }

+/**
+ * A DataShare is a tracked ARM resource modeled as a sub-resource of a Storage Account.


[NEW] 🟡 Warning [TSP — Misleading docstring on PATCH request body model]

Same issue as ConnectorUpdate: this docstring says:```typespec
/**

A DataShare is a tracked ARM resource modeled as a sub-resource of a Storage Account.
*/
model DataShareUpdate extends TrackedResourceUpdate { ... }```But DataShareUpdate is the PATCH request body, not a tracked ARM resource. Copy-pasted from the `DataShare` tracked resource.

Classification: [NEW] — DataShareUpdate was added in this PR.

Fix:```diff
-/**

A DataShare is a tracked ARM resource modeled as a sub-resource of a Storage Account.

/
+/*

The updatable properties of a Storage DataShare. Used as the body of the Update (PATCH) operation.

*/```

ravimeda · 2026-05-15T15:22:37Z

+}
+
+/**
+ * The resource model definition for an Azure Resource Manager tracked top level resource which has 'tags' and a 'location'


[NEW] 🟡 Warning [TSP — Local model shadows Azure.ResourceManager.CommonTypes.TrackedResourceUpdate + misleading description]

Two issues with this model:

1. Naming collision / shadowing. The local model is named TrackedResourceUpdate, which is the same simple name as the built-in Azure.ResourceManager.CommonTypes.TrackedResourceUpdate. Inside this TypeSpec project any future reference to the unqualified name TrackedResourceUpdate will silently resolve to this local definition instead of the common-type. This is a footgun for future maintainers who may expect to reference the standard library model.

2. Misleading description. The docstring says "has 'tags' and a 'location'" but this model only adds tags. location is not settable via PATCH — it comes from the parent Azure.ResourceManager.CommonTypes.Resource and should be treated as read-only in update payloads.

Classification: [NEW] — TrackedResourceUpdate was added in this PR.

Fix:

Rename the model to avoid the shadowing (e.g., StorageTrackedResourcePatch or StorageTrackedResourceUpdate), update the two callers (ConnectorUpdate extends ... and DataShareUpdate extends ...), and coordinate Go/JS/Python SDK breaking-change approvals for the definition rename.
If renaming is not desired due to additional SDK churn, add an inline comment explicitly noting the intentional shadowing so future maintainers don't accidentally unqualify a reference expecting the CommonTypes version.

Fix the description:

-/** - * The resource model definition for an Azure Resource Manager tracked top level resource which has 'tags' and a 'location' - */ +/** + * The patchable subset of a tracked ARM resource (— only resource tags). Used as a base for resource Update (PATCH) payloads. + */```

ravimeda · 2026-05-15T15:22:38Z

+  /**
+   * Test connection to backing data source before creating the storage connector.
+   */
+  @visibility(Lifecycle.Create, Lifecycle.Update)


[NEW] 💡 Suggestion [TSP — Unreachable Lifecycle.Create visibility on PATCH-only model]

@visibility(Lifecycle.Create, Lifecycle.Update) testConnection?: boolean = false;```This decorator is on a property of `StorageConnectorPropertiesUpdate`, which is **only used as the `PatchModel` of `ArmCustomPatchAsync`**. That means this model never appears in a Create body — `Lifecycle.Create` is unreachable here. This appears to be a copy-paste from the corresponding property in `StorageConnectorProperties` (where Create + Update visibility is appropriate). **Classification:** `[NEW]` — `StorageConnectorPropertiesUpdate` was added in this PR. **Fix (optional):** Drop the decorator entirely (the property is visible in all contexts by default, which is fine for a PATCH-only model) or narrow to `@visibility(Lifecycle.Update)` for documentation clarity: ```diff - @visibility(Lifecycle.Create, Lifecycle.Update) - testConnection?: boolean = false; + testConnection?: boolean = false;```

ravimeda · 2026-05-15T15:24:16Z

+model StorageConnectorPropertiesUpdate {
+  /**
+   * State - Active or Inactive. Whether or not the Storage Connector should start as active (default: Active)
+   * (While set to false on the Storage Connector, all data plane requests using this Storage Connector fail, and this Storage Connector is not billed if it would be otherwise.


[NEW] 💡 Suggestion [TSP — Unbalanced parenthesis in property description]

The description opens a parenthesis but never closes it:

/** * State - Active or Inactive. Whether or not the Storage Connector should start as active (default: Active) * (While set to false on the Storage Connector, all data plane requests using this Storage Connector fail, and this Storage Connector is not billed if it would be otherwise. */```The same issue exists in the corresponding `StorageConnectorProperties.state` (pre-existing); this PR carried it forward into the new `StorageConnectorPropertiesUpdate`. **Classification:** `[NEW]` for `StorageConnectorPropertiesUpdate` — this model was added in this PR. The same issue is `[EXISTING]` in `StorageConnectorProperties` (pre-existing). **Fix (optional):**```diff - * (While set to false on the Storage Connector, all data plane requests using this Storage Connector fail, and this Storage Connector is not billed if it would be otherwise. + * (While set to false on the Storage Connector, all data plane requests using this Storage Connector fail, and this Storage Connector is not billed if it would be otherwise.) ```

ravimeda · 2026-05-15T15:24:17Z

    emitter-output-dir: "{output-dir}/{service-dir}/arm-storage"
    service-dir: sdk/storage
    flavor: "azure"
+    compatibility-lro: true


[NEW] 💡 Suggestion [tspconfig — Consider documenting why compatibility-lro: true is set]

compatibility-lro: true```This setting preserves the pre-existing LRO method shape in the JavaScript SDK (`@azure/arm-storage`) that was already in GA before the TypeSpec migration. Without it, the `ArmCustomPatchAsync` template would change the emitted JavaScript LRO wrapper method names, causing a breaking change for existing callers. **Classification:** `[NEW]` — added in this PR. **Fix (optional):** Add a comment so future maintainers understand why this flag is set and can evaluate whether to keep it when bumping to a new API version: ```diff + # Preserve pre-existing LRO method signatures in @azure/arm-storage (GA SDK back-compat). compatibility-lro: true```

ravimeda · 2026-05-15T15:24:18Z

+@@clientName(
+  BlobServiceProperties.properties,
+  "BlobServiceProperties",
+  "!javascript"


[NEW] 💡 Suggestion [TSP — Consider adding a block comment explaining the !javascript scope pattern]

This PR adds "!javascript" to 9 @@clientName calls in this file (lines 199, 209, 224, 232, 243, 250, 275, 313, 325, 332) for the same reason: the JavaScript SDK already shipped these property names and renaming them would break existing callers. Adding a single block comment before the first such occurrence would document the convention once, instead of requiring readers to infer the pattern from 9 nearly-identical changes.

Classification: [NEW] — these scope qualifiers were added in this PR.

Fix (optional): Add a comment above this line:

+// The JS SDK (@azure/arm-storage) already shipped the property renames below under the original names. +// The "!javascript" scope ensures the renames apply to all other languages but leave the JS names unchanged. @@clientName( BlobServiceProperties.properties, "BlobServiceProperties", "!javascript" );```

ravimeda · 2026-05-15T15:27:56Z

[NEW] 🟡 Warning — Stable API version mutation with Go SDK breaking-change approval pending

This PR modifies two already-shipped stable API versions:

stable/2025-08-01/openapi.json
stable/2026-04-01/openapi.json

The PR carries the label BreakingChange-Go-Sdk but not BreakingChange-Go-Sdk-Approved. Under the Azure REST API repo policy, both stable-version changes and a Go SDK breaking-change require their respective approvals before merge.

Action required before merge:

Obtain the BreakingChange-Go-Sdk-Approved label (Go SDK team sign-off).
Confirm that the stable-version changes are intentional and have been reviewed by the API Review Board (or that the change is wire-identical and the stable-freeze exception applies).

Note: Wire-level inspection of 2025-08-01 confirms the PATCH payloads are byte-identical between BASE and HEAD — the ArmResourcePatchAsync → ArmCustomPatchAsync refactor is purely a TypeSpec source improvement. If the Go breaking-change flag was raised by tooling detecting the definition rename (Azure.ResourceManager.CommonTypes.TrackedResourceUpdate → TrackedResourceUpdate), please capture that reasoning in the PR description and request the approval label from the Go SDK team.

ravimeda · 2026-05-15T15:28:11Z

 };

+/**
+ * A Connector is a tracked ARM resource modeled as a sub-resource of a Storage Account.


[NEW] 🟡 Warning — Misleading doc-comment on ConnectorUpdate

The doc-comment says "The resource model definition for an Azure Resource Manager tracked top level resource…" — that is the boilerplate for a tracked ARM resource, but ConnectorUpdate is the PATCH request body (it only carries tags and properties). A reader scanning the generated SDK docs will think this is a full resource type.

Suggested fix: Replace the auto-generated tracked-resource blurb with something accurate, e.g.:

ypespec /** Request body for a PATCH update to a StorageConnector resource. */

ravimeda · 2026-05-15T15:28:30Z

 }

+/**
+ * A DataShare is a tracked ARM resource modeled as a sub-resource of a Storage Account.


[NEW] 🟡 Warning — Misleading doc-comment on DataShareUpdate

Same issue as ConnectorUpdate above: the boilerplate says "tracked top level resource" but this model is the PATCH body for a DataShare resource. Please replace with an accurate description, e.g.:

ypespec /** Request body for a PATCH update to a StorageDataShare resource. */

ravimeda · 2026-05-15T15:28:31Z

+}
+
+/**
+ * The resource model definition for an Azure Resource Manager tracked top level resource which has 'tags' and a 'location'


[NEW] 🟡 Warning — Misleading doc-comment and potential naming collision on TrackedResourceUpdate

Two issues:

Inaccurate description. The comment says "has a 'tags' and a 'location' property", but the model only has tags — there is no location property in a PATCH body (changing location on a tracked resource is not supported by ARM).

Naming collision risk. Azure.ResourceManager.CommonTypes already exports a type named TrackedResourceUpdate. Although the two definitions are currently shape-identical (tags: Record<string> + allOf to the base Resource), having a local type with the same name as an ARM common type can cause confusion for future maintainers and may silently shadow the common type if import order changes.

Suggested fixes:

Correct the description to mention only tags.

Consider a more specific name (e.g., StorageTrackedResourceUpdate) to avoid the naming overlap, or explicitly alias the common type instead.

ravimeda · 2026-05-15T15:28:50Z

+  /**
+   * Test connection to backing data source before creating the storage connector.
+   */
+  @visibility(Lifecycle.Create, Lifecycle.Update)


[NEW] 💡 Suggestion — Dead Lifecycle.Create visibility on testConnection

testConnection is defined inside StorageConnectorPropertiesUpdate, which is the PATCH-only request body. The visibility is set to @visibility(Lifecycle.Create, Lifecycle.Update), but Lifecycle.Create is unreachable here — a PATCH body is never used in a PUT/create operation.

Consider using @visibility(Lifecycle.Update) only, or simply omitting the decorator (the default for a PATCH-body property already scopes to update).

ravimeda · 2026-05-15T15:28:51Z

+model StorageConnectorPropertiesUpdate {
+  /**
+   * State - Active or Inactive. Whether or not the Storage Connector should start as active (default: Active)
+   * (While set to false on the Storage Connector, all data plane requests using this Storage Connector fail, and this Storage Connector is not billed if it would be otherwise.


[NEW] 💡 Suggestion — Unbalanced parenthesis in state description (pre-existing, carried into new model)

The description for state reads: "The state of the storage connector (either Active or Inactive" — the closing parenthesis ) is missing.

Suggested fix:
ypespec /** The state of the storage connector (either Active or Inactive). */

This is a pre-existing issue in StorageConnectorProperties that has been copied into the new StorageConnectorPropertiesUpdate model; worth fixing in both places.

ravimeda · 2026-05-15T15:29:11Z

    emitter-output-dir: "{output-dir}/{service-dir}/arm-storage"
    service-dir: sdk/storage
    flavor: "azure"
+    compatibility-lro: true


[NEW] 💡 Suggestion — Add a rationale comment for compatibility-lro: true

compatibility-lro: true is a non-obvious emitter knob. Future maintainers (and SDK reviewers) will not know why it was enabled without context. Please add a brief inline comment, for example:

yaml # Required for JavaScript SDK back-compat: preserves the pre-LRO polling behavior # that existing callers depend on. Remove only after coordinating a major-version bump. compatibility-lro: true

ravimeda · 2026-05-15T15:29:12Z

+@@clientName(
+  BlobServiceProperties.properties,
+  "BlobServiceProperties",
+  "!javascript"


[NEW] 💡 Suggestion — Add a block comment explaining the "!javascript" scope pattern

This PR introduces nine @@clientName(..., "!javascript") overrides, but there is no comment explaining what the scope filter does or why it is applied here. Readers unfamiliar with the @azure-tools/typespec-client-generator-core scoping feature will not understand the intent.

Suggested comment to add above the first occurrence (or near the top of the file):

ypespec // The "!javascript" scope excludes the JavaScript/TypeScript emitter from the // @@clientName rename below, preserving the original auto-generated name for // the JS SDK to maintain back-compatibility with existing callers.

blueww

It looks good to me, tested with .net SDK, the update datashare/ dataconnector works in .net SDK API now.

BTW, I add API version 2025-08-01 to .net SDK config.

#38586) Configurations: 'specification/storage/Storage.Management/tspconfig.yaml', and CommitSHA: 'b373ded4a6c77a9f541ca8f020fd2072db632751' in SpecRepo: 'https://github.com/Azure/azure-rest-api-specs' Pipeline run: https://dev.azure.com/azure-sdk/internal/_build/results?buildId=6318750 Refer to https://eng.ms/docs/products/azure-developer-experience/develop/sdk-release/sdk-release-prerequisites to prepare for SDK release. **Release plan link:** [https://azsdk-releaseplan-dashboard-hveph5aqhhcfhtgu.westus-01.azurewebsites.net/?releaseplan=2220](https://azsdk-releaseplan-dashboard-hveph5aqhhcfhtgu.westus-01.azurewebsites.net/?releaseplan=2220) **Submitted by**: weiwei@microsoft.com ## Release Plan Details - Release Plan: https://aka.ms/sdk-release-planner?release-plan-id=3044af4d-f349-f111-bec7-000d3a5c46b0 Spec pull request: Azure/azure-rest-api-specs#43207 Spec API version: --------- Co-authored-by: kazrael2119 <98569699+kazrael2119@users.noreply.github.com>

Use ArmCustomPatch template

d252a83

pshao25 marked this pull request as ready for review May 14, 2026 10:54

pshao25 requested review from blueww and yifanz7 as code owners May 14, 2026 10:54

microsoft-github-policy-service Bot added the Storage Storage Service (Queues, Blobs, Files) label May 14, 2026

github-actions Bot added resource-manager RPaaS TypeSpec Authored with TypeSpec labels May 14, 2026

github-actions Bot added BreakingChange-Go-Sdk BreakingChange-JavaScript-Sdk BreakingChange-Python-Sdk labels May 14, 2026

Merge branch 'main' into storageFix

23da95d

kazrael2119 added 2 commits May 15, 2026 15:10

update js config and format

66b5ea9

Update models.tsp

76a7ada

msyyc added the BreakingChange-Python-Sdk-Approved label May 15, 2026

kazrael2119 added 2 commits May 15, 2026 15:46

Update models.tsp

fdef77d

Update models.tsp

563fa78

github-actions Bot added WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required and removed ARMAutoSignedOff-IncrementalTSP ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review labels May 15, 2026

kazrael2119 added the BreakingChange-JavaScript-Sdk-Approved label May 15, 2026

ravimeda reviewed May 15, 2026

View reviewed changes

ravimeda added ARMChangesRequested and removed WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels May 15, 2026

ravimeda reviewed May 15, 2026

View reviewed changes

ujjawaljain-msft approved these changes May 16, 2026

View reviewed changes

Add 2025-08-01 version tag to .net SDK

5db615f

github-actions Bot added ARMAutoSignedOff-IncrementalTSP ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review and removed ARMChangesRequested labels May 18, 2026

blueww approved these changes May 18, 2026

View reviewed changes

pshao25 added the PublishToCustomers Acknowledgement the changes will be published to Azure customers. label May 18, 2026

tadelesh added the BreakingChange-Go-Sdk-Approved label May 18, 2026

tadelesh approved these changes May 18, 2026

View reviewed changes

blueww merged commit ed14512 into main May 18, 2026
79 checks passed

blueww deleted the storageFix branch May 18, 2026 09:46

azure-sdk mentioned this pull request May 19, 2026

[AutoPR @azure-arm-storage]-generated-from-SDK Generation - JS-6318750 Azure/azure-sdk-for-js#38586

Merged

baywet mentioned this pull request May 27, 2026

refresh from main #43551

Closed

Uh oh!

Conversation

pshao25 commented May 14, 2026

Choose a PR Template

Uh oh!

github-actions Bot commented May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Next Steps to Merge

Uh oh!

github-actions Bot commented May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

API Change Check

Uh oh!

ravimeda commented May 15, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ravimeda commented May 15, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blueww left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

github-actions Bot commented May 14, 2026 •

edited

Loading

github-actions Bot commented May 14, 2026 •

edited

Loading

blueww left a comment •

edited

Loading