Skip to content

Migrate Sql to support TypeSpec#59913

Merged
ArthurMa1978 merged 28 commits into
mainfrom
mgmt-sql
Jun 24, 2026
Merged

Migrate Sql to support TypeSpec#59913
ArthurMa1978 merged 28 commits into
mainfrom
mgmt-sql

Conversation

@ArthurMa1978

Copy link
Copy Markdown
Member

Fixes #59028
Spec pr: Azure/azure-rest-api-specs#43954

Contributing to the Azure SDK

Please see our CONTRIBUTING.md if you are not familiar with contributing to this repository or have questions.

For specific information about pull request etiquette and best practices, see this section.

Copilot AI review requested due to automatic review settings June 14, 2026 17:01

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 wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

@github-actions github-actions Bot added the SQL label Jun 14, 2026
ArthurMa1978 and others added 11 commits June 15, 2026 17:16
Adds custom partial classes under src/Custom/ to mitigate the breaking
changes that were visible in the truncated ApiCompat output:

- DistributedAvailabilityGroup{Data,Resource,Collection} wrappers for
  the Sql-prefix rename
- SqlDatabaseBlobAuditingPolicyCollection singleton-to-collection shim
- SqlDatabaseCollection.Get/Exists(string,ct) overloads
- ManagedDatabaseSensitivityLabel*, ManagedDatabaseColumn*,
  SqlDatabaseColumn* recommendation method shims
- SqlDatabaseResource.{CancelDatabaseOperation,
  CreateOrUpdateDatabaseExtension, CreateRestorePoint} stubs
- SqlDatabaseBlobAuditingPolicyResource.{Update, CreateResourceIdentifier}
- ManagedInstanceResource.GetTopQueries(options) overload and
  GetDistributedAvailabilityGroup* forwarders
- RestorableDroppedManagedDatabaseResource tag NotSupported stubs
- ManagedInstanceQuery, ManagedInstanceResourceGetTopQueriesOptions
  models
- PageableHelpers internal helper

Compile is clean. ApiCompat against v1.4.0 still reports ~1006 unique
breakages (8 entire missing resource families plus removed members,
sealed types, non-virtual properties, removed WirePathAttribute), all
requiring either upstream TypeSpec changes plus a regeneration cycle,
or a v2.0 major-version bump.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…and setters

Adds 42 @@Usage(<Model>, Usage.input, csharp) directives in client.tsp for output-only models that lost their parameterless ctor and writable setters in the new TypeSpec generation. Fixes ~150 ApiCompat regressions (459 -> 309 unique errors).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds 6 @@clientName operation renames in client.tsp to restore v1.4.0 method names: GetDeletedServers, GetSyncDatabaseIdsSyncGroups, and 4 sensitivity-label operations on SqlDatabaseResource. Drops ApiCompat from 309 to 303 unique errors.

Note: 5 additional LongTermRetention pageable renames were attempted but caused a generator bug (CollectionResultOfT ctor arg mismatch); reverted.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@ArthurMa1978 ArthurMa1978 requested review from a team and jsquire as code owners June 18, 2026 09:45

@live1206 live1206 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.

Phase 1 (Versioning): package remains 1.5.0-beta.1, ApiCompatVersion remains 1.4.0, and the new ApiCompat baseline entries are limited to targeted WirePathAttribute removals with justification. No versioning blocker found.

Phase 2 (API Review): ran the trusted management naming scanner against the PR API and released Azure.ResourceManager.Sql_1.4.0 API. Scanner found one deterministic bool naming issue (ManagedInstanceDtcData.FqdnEnabled). Contextual naming: evaluated 14 new public types, flagged 4 migration duplicate/drift types (SqlDataMaskingFunction, SqlDataMaskingRuleState, SyncGroupsType, DevOpsAuditingSettingsName).

Phase 3 (Breaking Change Detection): PR CI is green and no unapproved ApiCompat suppressions were found beyond the allowed WirePathAttribute removals. I did not run PR code locally.

Phase 4 (Migration Customization): flagged ModelFactory direct internal-constructor usage and hide-and-replace migration shims where generated APIs should be renamed/suppressed instead. Other reviewed customization blocks generally include compatibility/generator-issue justification comments.

Phase 5 (Decorator Preference): flagged generated-name drift that should be fixed in TypeSpec/client customization where possible (@@clientName(..., "csharp") / alternate type/suppression as applicable) rather than maintaining duplicate SDK shims.

Phase 6 (New API Triage): accepted real additions include the new Managed Instance Query resource trio and identity/sensitivity-label/TDE/vulnerability-assessment supporting types. Flagged rename/drift count: 4 public types. Duplicate model/enum/operation counts: 3 duplicate enum/type shapes and 2 duplicate operation/overload groups.

Final counts: critical issues 0, should-fix issues 5, suggestions 0. Total inline comment count: 5.

@live1206 live1206 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.

Additional pass after the initial migration review: I found one more source-compatibility issue affecting named arguments on the long-term-retention access-tier methods. Existing findings from the previous review still stand; this follow-up adds 2 inline comments for the same issue in the resource-group and subscription variants.

Comment thread sdk/sqlmanagement/Azure.ResourceManager.Sql/src/Custom/Models/QueryStatistics.cs Outdated

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Management SDK Review Summary

  • Scope: Azure.ResourceManager.Sql (migration PR: Swagger/AutoRest → TypeSpec)
  • Versioning: ✅ Pass — version 1.5.0-beta.1, ApiCompatVersion 1.4.0 present. ApiCompat baseline contains only justified WirePathAttribute removals on ManagedInstanceResourceGetTopQueriesOptions properties (no functional impact; allowed per migration rules).
  • API surface: ✅ Pass — 13 new public types evaluated; no new deterministic naming violations found. Previously flagged issues (SqlDataMaskingFunction/DataMaskingFunction duplicate, SqlDataMaskingRuleState/DataMaskingRuleState duplicate, DevOpsAuditingSettingsName) were all reviewed, resolved, and accepted by the human reviewer.
  • Contextual naming: evaluated 13 new public types (CurrentSensitivityLabelSource, DataSyncParticipantIdentity, DataSyncParticipantIdentityType, DataSyncParticipantUserAssignedIdentity, DevOpsAuditingSettingsName, ManagedInstanceQueryCollection, ManagedInstanceQueryData, ManagedInstanceQueryResource, RecommendedSensitivityLabelSource, SqlDataMaskingFunction, SqlDataMaskingRuleState, TdeCertificateProperties, VulnerabilityAssessmentSystemDatabaseName), flagged 0 new issues.
  • ApiCompat / breaking changes: ⏳ CI in progress — no failures detected at review time. ApiCompat baseline entries are limited to targeted WirePathAttribute removals.
  • Migration-specific checks (Phases 4–6): ✅ Pass — human reviewer @live1206 conducted a full migration review (Phases 1–6) across all 2,657 changed files. All 15 review threads are resolved. PR approved by @live1206 on the current head commit (973b599).

Non-blocking note: CHANGELOG.md has no content in the 1.5.0-beta.1 section (Features Added / Breaking Changes / Other Changes are all blank). For a large migration PR this should be filled in before release to document the API changes introduced by the TypeSpec migration. No action required to unblock merge.

No new management SDK review findings on this head commit. The comprehensive review has already been completed by @live1206.


Analyzed by management-sdk-review workflow: azure-sdk-mgmt-pr-review

Analyzed by Azure .NET Management SDK PR Review: https://github.com/Azure/azure-sdk-for-net/actions/runs/28088944405

@ArthurMa1978 ArthurMa1978 merged commit a29f67e into main Jun 24, 2026
31 checks passed
@ArthurMa1978 ArthurMa1978 deleted the mgmt-sql branch June 24, 2026 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

** MPG migration - sql

3 participants