[StorageCache] Add RebalanceJob sub-resource for API version 2026-08-01#43757
[StorageCache] Add RebalanceJob sub-resource for API version 2026-08-01#43757brpanask wants to merge 7 commits into
Conversation
Next Steps to MergeNext steps that must be taken to merge this PR:
Comment generated by summarize-checks workflow run. |
API Change CheckAPIView identified API level changes in this PR and created the following API reviews
Comment generated by After APIView workflow run. |
524cd95 to
ab0f9d5
Compare
Adds TypeSpec version entry for Microsoft.StorageCache API version 2026-08-01. No content changes - baseline identical to 2026-01-01.
…-08-01 - New RebalanceJob.tsp: TrackedResource with GET, PATCH (cancel via adminStatus), DELETE, and LIST operations. No PUT (creation via Expansion only). - models.tsp: RebalanceJobProperties, RebalanceJobPropertiesStatus, RebalanceJobUpdate, RebalanceJobUpdateProperties, RebalanceJobsListResult, RebalanceJobPropertiesProvisioningState, RebalanceJobStatusType, RebalanceJobAdminStatus unions. - ExpansionJobProperties: added rebalance (bool, create-only) and rebalanceJobName (string, read-only) with @added(Versions.v2026_08_01). - main.tsp: import RebalanceJob.tsp. - Examples for all 4 RebalanceJob operations + updated expansion GET example. - Generated openapi.json for 2026-08-01 updated (2026-01-01 unchanged). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
c700350 to
7c17933
Compare
b18fbcd to
75aa5f0
Compare
LintDiff Failures - All Inherited, Not From RebalanceJobThe Swagger LintDiff check shows ~1000 violations, but none are from the new RebalanceJob resource. All violations are pre-existing patterns from existing resources (AmlFilesystem, ImportJob, ExportJob, Cache, StorageTarget, etc.) that appear as "new" because the \stable/2026-08-01/openapi.json\ file is entirely new to this repo. Violation breakdown (all from existing resources):
What this PR actually adds:
Request:Requesting \Approved-LintDiff\ label since all violations are inherited from the existing TypeSpec conversion (same patterns exist in \stable/2026-01-01/openapi.json\ on main). Stale labels:The \BreakingChange-JavaScript-Sdk\ and \BreakingChange-Python-Sdk\ labels are stale (from a previous iteration that used common-types v6). Breaking change checks now pass — please remove these labels. |
- New ProxyResource: Microsoft.StorageCache/amlFilesystems/rebalanceJobs - Operations: Get, List, Update (PATCH), Delete - Uses common-types v6 - Examples with PascalCase operationIds Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
75aa5f0 to
e9e01e7
Compare
LintDiff "New Violations" — False Positive from
|
…balanceJob'; fix RebalanceJobs_Delete async headers to ascOperations form Renames ExpansionJobProperties.rebalance -> runRebalanceJob (avoids bare-noun boolean, matches RP internal name) and regenerates openapi.json. Corrects the RebalanceJobs_Delete 202 Azure-AsyncOperation/Location headers to the subscription-scoped ascOperations form emitted by Compute-CPlat-Core, consistent with importJobs_Delete.
…out mebibytes per second
Removed x-ms-mutability extensions (and their no-openapi suppressions) from ResourceSkuLocationInfo.location and StorageTarget.location. Both suppressions cited the R4002 LocationMustHaveXmsMutability rule, which applies only to a *tracked* resource's location property. ResourceSkuLocationInfo is a plain model and StorageTarget is a ProxyResource, so R4002 never applied to either. This reverts the ResourceSkuLocationInfo.location mutability constraint that had regenerated into the already-shipped stable/2026-01-01 GA contract, clearing the inert breaking-change flagged in review. Regenerated OpenAPI for both 2026-01-01 and 2026-08-01; tsp compile passes with no errors or warnings.
|
if the lintdiff checks are failing for existing operations/RT's then they must have been failing for previous versions. Can you provide the pr and the exception that would have been provided before? |
|
also if there are violations this is a good opportunity to fix them. |
|
Thanks @psah434. Before the specifics, some context: StorageCache was migrated to TypeSpec by the ARM TypeSpec conversion team — that team authored the On your two points: Prior PR + exception: This is the same pattern approved in #38978 ("StorageCache base commit for API version 2026-01-01"), which received On fixing: Separately, could the stale |
|
Context on the outstanding LintDiff findings These LintDiff results are inherited from the ARM/TypeSpec team's own brownfield conversion of StorageCache, not introduced by this feature change. TypeSpec conversion PR: #42197 — "[TSP Migration] storagcache TypeSpec migrated from swagger" (conversion wave On On |
psah434
left a comment
There was a problem hiding this comment.
ARM API Review
Posting findings from the ARM API Reviewer agent (critic-verified, 2 iterations, converged) against commit bb6da1a. See inline comments for findings 1-7. No blocking issues were found; the change is additive and non-breaking against 2026-01-01.
| */ | ||
| @visibility(Lifecycle.Read) | ||
| @pageItems | ||
| @identifiers(#["name"]) |
There was a problem hiding this comment.
[NEW] 🟠 Warning [TSP @added misalignment] / uniform-versioning — un-gated models.tsp edits regenerate the already-released stable/2026-01-01 spec.
Classification reasoning: Introduced in this PR. This @identifiers(#["name"]) addition and the doc-comment additions (replacing removed documentation-required FIXME suppressions) are not gated by @added(Versions.v2026_08_01), so they flow into the previously-released 2026-01-01 generated spec — adding descriptions to 5 definitions (AutoExportJobUpdateProperties, AutoImportJobUpdateProperties, ImportJobUpdateProperties, MetricAggregationType, userAssignedIdentitiesValue) and x-ms-identifiers: ["name"] to ResourceSkusResult.value (stable/2026-01-01/openapi.json line 7982).
Issue: Descriptions are non-breaking, but modifying a GA version's contract should be intentional, and the x-ms-identifiers addition changes SDK array handling for that GA version.
Suggested fix: Confirm the 2026-01-01 changes are intended. If not, gate these additions to 2026-08-01; otherwise accept them as documented non-breaking improvements. Ensure the oad breaking-change CI passes on the modified 2026-01-01 spec.
| * Average throughput in mebibytes per second (1024x1024 bytes per second) over the most recent reporting interval. | ||
| */ | ||
| @visibility(Lifecycle.Read) | ||
| throughputMbps?: float64; |
There was a problem hiding this comment.
[NEW] 🔵 Suggestion [TSP-NUMERIC-TYPE / units clarity] — throughputMbps?: float64.
Classification reasoning: New property added in this PR.
Issue: The name throughputMbps conventionally implies megabits per second, but the doc comment says mebibytes per second (1024×1024 bytes/sec). This is ambiguous for SDK consumers.
Suggested fix: Consider throughputMiBps (or a clearer name). It is a new read-only property, so renaming now is cheap. Low severity since the units are documented.
| /** | ||
| * The time (in UTC) the rebalance job started. | ||
| */ | ||
| #suppress "@azure-tools/typespec-azure-core/casing-style" "Non-standard casing matches existing convention in this project" |
There was a problem hiding this comment.
[NEW] 🔵 Suggestion [Suppression justification §4.1] — casing-style suppressions on startTimeUTC / completionTimeUTC (this line and line 2578).
Classification reasoning: New suppressions added in this PR.
Issue: The reason "Non-standard casing matches existing convention in this project" is a consistency-based justification, which Section 4.1 discourages. The casing itself is defensible given the established project-wide *TimeUTC convention.
Suggested fix: Reword the reason to state the concrete technical rationale (SDK property-name consistency with the sibling *TimeUTC properties across the StorageCache job-resource family).
| #suppress "@azure-tools/typespec-azure-core/no-legacy-usage" "FIXME: Update justification, follow aka.ms/tsp/conversion-fix for details" | ||
| @@Legacy.flattenProperty(ExpansionJob.properties); | ||
|
|
||
| #suppress "@azure-tools/typespec-azure-core/no-legacy-usage" "Required for ProvisioningStateMustBeReadOnly lint rule to traverse properties" |
There was a problem hiding this comment.
[NEW] 🔵 Suggestion [@flattenProperty on new APIs §5] — @@Legacy.flattenProperty(RebalanceJob.properties) with a no-legacy-usage suppression.
Classification reasoning: New resource (RebalanceJob) added in this PR; this legacy flatten + suppression are new.
Issue: New resource types should avoid legacy property flattening (SDK-breaking risk). The suppression reason ("Required for ProvisioningStateMustBeReadOnly lint rule to traverse properties") is technically questionable — flattening is not generally required for that rule.
Suggested fix: Verify whether flattening is truly required. If kept for consistency with the sibling job resources, document that concrete rationale in the suppression reason. Low severity given family consistency.
|
|
||
| @added(Versions.v2026_08_01) | ||
| @armResourceOperations(#{ omitTags: true }) | ||
| interface RebalanceJobs { |
There was a problem hiding this comment.
[NEW] 🔵 Suggestion [ARM job-cancellation pattern] — RebalanceJobs exposes GET/LIST/PATCH/DELETE, no PUT; cancellation is via PATCH adminStatus: Cancel.
Classification reasoning: New resource interface added in this PR.
Issue: A dedicated POST cancel action is the more idiomatic ARM pattern for cancelling a job, versus overloading PATCH with an adminStatus field.
Suggested fix: Optional / design observation only — PATCH-adminStatus matches this project's existing job-resource family (ImportJob, AutoExportJob, etc.), so keeping it for consistency is reasonable. No PUT is correct here since RebalanceJob is system-created via the expansion runRebalanceJob flag.
| * Returns a rebalance job. | ||
| */ | ||
| @tag("rebalanceJobs") | ||
| get is ArmResourceRead<RebalanceJob, Error = CloudError>; |
There was a problem hiding this comment.
[NEW] 🔵 Suggestion [Consistent error responses §2.7] — mixed error models within the same interface.
Classification reasoning: New resource interface added in this PR.
Issue: get (this line) and listByAmlFilesystem (line 74) use Error = CloudError, while update (line 51) and delete (line 64) use Error = ErrorResponse. Mixed error models across operations of the same resource produce inconsistent error schemas in the generated spec and SDKs.
Suggested fix: Use a single consistent error model across all operations of the RebalanceJobs interface.
| * The administrative status of the rebalance job. Possible values: 'Active', 'Cancel'. Passing in a value of 'Cancel' will cancel the current active rebalance job. By default it is set to 'Active'. | ||
| */ | ||
| @visibility(Lifecycle.Read, Lifecycle.Create) | ||
| adminStatus?: RebalanceJobAdminStatus = RebalanceJobAdminStatus.Active; |
There was a problem hiding this comment.
[NEW] 🔵 Suggestion [Visibility / mutability §2.3] — adminStatus?: RebalanceJobAdminStatus = RebalanceJobAdminStatus.Active with @visibility(Lifecycle.Read, Lifecycle.Create).
Classification reasoning: New property added in this PR.
Issue: RebalanceJob has no PUT/create operation, so the Lifecycle.Create visibility and the default value are unused in request contexts — the only writable path is PATCH via RebalanceJobUpdateProperties.adminStatus.
Suggested fix: Consider dropping the Lifecycle.Create visibility and the default for clarity. Low severity.
Reviewer Guidance (please read first)
Status: 32/33 required checks green. This PR is ready for ARM review. Three items need a reviewer/maintainer with write access:
Approved-LintDiff— Swagger LintDiff is the only red check. The 2 "new" errors areProvisioningStateMustBeReadOnlyfalse positives from Spectral$refsibling resolution (OpenAPI 2.0 drops the siblingreadOnly: truewhen inlining the$ref).provisioningStateis correctlyreadOnly: true; the same rule fires on every existing resource (AmlFilesystem, ExpansionJob, ImportJob, Cache, StorageTarget, …) and across other TypeSpec services (apicenter, advisor, standbypool). Only the 2 RebalanceJob entries are classified "new" because the resource path is new. Full before/after breakdown in the comment dated 2026-07-01. RequestingApproved-LintDiff.BreakingChange-JavaScript-SdkandBreakingChange-Python-Sdk— left over from an earlier common-types-v6 iteration. The erroneousx-ms-mutabilityonlocationwas removed and BreakingChange now passes; these labels no longer apply.ARMSignedOff— PR is inWaitForARMFeedback; needs an ARM reviewer sign-off.PublishToCustomerswill be added by us only when we're ready to release to customers (per the Next-Steps-to-Merge bot comment).API Info
Service: StorageCache (Microsoft.StorageCache)
API Version: 2026-08-01
Type: Stable
Changes
Adds the RebalanceJob sub-resource to AMLFS and new expansion-triggered rebalance fields. New API version building on 2026-01-01.
New Resource: RebalanceJob
New Fields
runRebalanceJobopt-in property on the expansion (when set, the expansion creates a RebalanceJob after completing);rebalanceJobNameread-only back-referenceSDK Compatibility
Design Notes
RebalanceJob has no PUT/create operation (intentional)
A RebalanceJob is created as a side-effect of an ExpansionJob (when the rebalance opt-in on the expansion is set), not by a customer-initiated PUT. It therefore exposes GET, LIST, PATCH, and DELETE only:
adminStatus(Active->Cancel).provisioningState(read-only) is present because the resource supports asynchronous PATCH/DELETE per RPC-Async-V1-02. This is a read-and-manage resource rather than a create-via-PUT resource, so the absence of a PUT is by design, not an asymmetric-CRUD omission.RebalanceJob is a ProxyResource (no tags/location)
RebalanceJob is modeled as a
ProxyResource(proxy in the ARM manifest), so it has notags/locationenvelope.Checklist
Breaking Changes
None - all additive.