Skip to content

Conversation

@Yuqing-cat
Copy link
Member

@Yuqing-cat Yuqing-cat commented Oct 7, 2025

ARM (Control Plane) API Specification Update Pull Request

Tip

Overwhelmed by all this guidance? See the Getting help section at the bottom of this PR description.

PR review workflow diagram

Please understand this diagram before proceeding. It explains how to get your PR approved & merged.

spec_pr_review_workflow_diagram

Purpose of this PR

What's the purpose of this PR? Check the specific option that applies. This is mandatory!

  • New resource provider.
  • New API version for an existing resource provider. (If API spec is not defined in TypeSpec, the PR should have been created in adherence to OpenAPI specs PR creation guidance).
  • Update existing version for a new feature. (This is applicable only when you are revising a private preview API version.)
  • Update existing version to fix OpenAPI spec quality issues in S360.
  • Convert existing OpenAPI spec to TypeSpec spec (do not combine this with implementing changes for a new API version).
  • Other, please clarify:
    • edit this with your clarification

The changes have been reviewed in PrPr version:

Introduces a new computeMode property and improves flexibility in compliance definitions

  • Introduces a required computeMode property with two supported values: Hybrid or Serverless.
  • Updates managesResourceGroupId to be optional, as this value should be omitted for Serverless workspaces.
  • Update the detailed ComplianceStandardDefinition Enum section to add more supported Compliance Standards.

Due diligence checklist

To merge this PR, you must go through the following checklist and confirm you understood
and followed the instructions by checking all the boxes:

  • I confirm this PR is modifying Azure Resource Manager (ARM) related specifications, and not data plane related specifications.
  • I have reviewed following Resource Provider guidelines, including
    ARM resource provider contract and
    REST guidelines (estimated time: 4 hours).
    I understand this is required before I can proceed to the diagram Step 2, "ARM API changes review", for this PR.
  • A release plan has been created. If not, please create one as it will help guide you through the REST API and SDK creation process.

@github-actions
Copy link

github-actions bot commented Oct 7, 2025

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 github-actions bot added brownfield Brownfield services will soon be required to convert to TypeSpec. See https://aka.ms/azsdk/typespec. ARMReview new-api-version resource-manager WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Oct 7, 2025
@github-actions
Copy link

github-actions bot commented Oct 7, 2025

API Change Check

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

Language API Review for Package
JavaScript @azure/arm-databricks
Swagger Microsoft.Databricks-Databricks

@Yuqing-cat Yuqing-cat marked this pull request as draft October 7, 2025 22:44
@github-actions github-actions bot added BreakingChangeReviewRequired <valid label in PR review process>add this label when breaking change review is required NotReadyForARMReview and removed WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Oct 7, 2025
…review' of https://github.com/Yuqing-cat/azure-rest-api-specs into dev/yuqing/databricks-Microsoft.Databricks/2025-10-01-preview
@Yuqing-cat Yuqing-cat marked this pull request as ready for review October 7, 2025 22:47
Comment on lines +1174 to +1177
"x-ms-enum": {
"modelAsString": true,
"name": "ComplianceStandard"
}
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure it makes sense to have an x-ms-enum on a property that is not an enum.

Copy link
Member Author

@Yuqing-cat Yuqing-cat Oct 20, 2025

Choose a reason for hiding this comment

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

Thanks for catching this, Mike. We'd like to keep the name "ComplianceStandard", so we don't remove this part.

@mikekistler mikekistler added the BreakingChange-Approved-Benign Changes are not breaking at the REST API level and have at most minor impact to generated SDKs. label Oct 20, 2025
@github-actions github-actions bot added WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required and removed NotReadyForARMReview labels Oct 20, 2025
}
}
},
"ComplianceStandardDefinition": {
Copy link
Contributor

Choose a reason for hiding this comment

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

ComplianceStandardDefinition

is this not being an enum now ? Will it take any free form string now ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi @ramoka178 , this will not accept arbitrary free-form strings. The implementation expects an extensible list of predefined string values.
We simplified this to a string rather than maintaining an explicit enum in Swagger because the set of compliance standards evolves rapidly and is owned by a partner team. Using enums would require creating new API versions every time a value is added, which increases engineering overhead and makes it harder for customers to continuously upgrade.
Please let us know if you have a better approach to achieve flexibility without introducing free-form input.

Copy link
Contributor

Choose a reason for hiding this comment

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

If the variable accepts a set of predefined values, you should implement the enums. I understand for every new value, the swagger should be updated. But , please think of the end user's experience. If you publish the expected values in the swagger, the user will already know what values are expected . If you chose to stop supporting an enum, via a breaking change approval, that can also be published and the user will be aware of it.

If there is no insight to the customer, would they be just randomly trying out different values until they find the right one ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi @ramoka178, thank you for raising this concern—it’s a valid point.
For context:

  • This feature is mainly used by enterprise customers with strict compliance needs. They onboard through a partner-managed process, and we’ve linked the official documentation for clarity.
  • In the past six months, we’ve added four new standards. Each time, customers requested immediate access for compliance reasons and required public preview before adoption.
  • Updating Enums in a published preview spec is considered a breaking change, which forces new API versions or delays with batch releases—both harm customer experience.

We discussed this in Breaking Change Office Hours, and the approved guidance was to use string for flexibility. While not ideal, this avoids repeated version churn and ensures faster delivery for customers.

If there’s a better approach that balances discoverability and forward compatibility, we’re open to suggestions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Addition of enums to an existing list should not trigger breaking changes I believe. Removing of Enums might do that IIRC.
And, adding of enums can still happen in the same api-version.

I believe this is a standard practice of using Enums across Azure i.e., creating PRs for the new enums and those should go in without a new api-version.

If you make it a free form string, there could be multiple issues which are both client and server impacting.

  1. You lose server-side schema validation
  2. Users will need to consult the documentation all the time to see what values are valid, results in Poor API contract clarity
  3. In SDKs, enums are converted to constants I believe. you would lose on that.
  4. You would need some server-side normalization to account for case issues

Using enums is cleaner and easier to adopt. I agree that it might make development lifecycle longer than usual, but I would argue to keep the customer experience as a top priority.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you for sharing your detailed feedback—it’s greatly appreciated.
Our top priority is customer experience, and the original intent behind proposing Enum removal was to help customers adopt new compliance standards faster without repeated API version upgrades, which has been a major pain point.
We respect your guidance and have reverted the change to align with ARM expectations. At the same time, we will continue to explore ways to improve agility while maintaining clarity and validation, as this remains critical for enterprise customers.

@github-actions github-actions bot added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Oct 21, 2025
@ramoka178
Copy link
Contributor

Please fix the Swagger LintDiff error too

@ramoka178 ramoka178 added ARMChangesRequested and removed WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Oct 21, 2025
@Yuqing-cat
Copy link
Member Author

Please fix the Swagger LintDiff error too

The LintDiff error is inherited from previous API versions. The property KeyName has been long existing in the Databricks stable and preview API version.

I attempted to fix this in earlier commits, but it triggered a breaking-change violation. The breaking-change review team requested that we revert the change to avoid impacting existing clients.
image

I'm adding a suppression directive with justification in readme.md to bypass this.

More details are listed below:
The must fix LintDiff Error:
image
Is implemented in default (2025-03-01-preivrew) API version and suggested to be kept as is per breaking change review.
image

@Yuqing-cat Yuqing-cat added WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required and removed ARMChangesRequested labels Oct 21, 2025
@ramoka178 ramoka178 added ARMChangesRequested and removed WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Oct 21, 2025
@Yuqing-cat Yuqing-cat added WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required and removed ARMChangesRequested labels Oct 21, 2025
@ramoka178 ramoka178 added Approved-Suppression ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review and removed WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Oct 22, 2025
@github-actions github-actions bot removed the brownfield Brownfield services will soon be required to convert to TypeSpec. See https://aka.ms/azsdk/typespec. label Oct 23, 2025
@Yuqing-cat Yuqing-cat added the PublishToCustomers Acknowledgement the changes will be published to Azure customers. label Oct 24, 2025
@Yuqing-cat Yuqing-cat merged commit 75a477d into Azure:main Oct 24, 2025
49 of 50 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Approved-Suppression ARMReview ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review BreakingChange-Approved-Benign Changes are not breaking at the REST API level and have at most minor impact to generated SDKs. BreakingChangeReviewRequired <valid label in PR review process>add this label when breaking change review is required new-api-version PublishToCustomers Acknowledgement the changes will be published to Azure customers. resource-manager SuppressionReviewRequired

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants