Skip to content

Updated Swagger as per latest changes.#8427

Merged
yungezz merged 25 commits into
Azure:masterfrom
amoghnatu:users/amnat/databricks_SwaggerSpecs_Update
Apr 7, 2020
Merged

Updated Swagger as per latest changes.#8427
yungezz merged 25 commits into
Azure:masterfrom
amoghnatu:users/amnat/databricks_SwaggerSpecs_Update

Conversation

@amoghnatu

@amoghnatu amoghnatu commented Feb 19, 2020

Copy link
Copy Markdown
Contributor

Latest improvements:

MSFT employees can try out our new experience at OpenAPI Hub - one location for using our validation tools and finding your workflow.

Contribution checklist:

  • I have reviewed the documentation for the workflow.
  • Validation tools were run on swagger spec(s) and have all been fixed in this PR.
  • The OpenAPI Hub was used for checking validation status and next steps.

ARM API Review Checklist

  • Service team MUST add the "WaitForARMFeedback" label if the management plane API changes fall into one of the below categories.
  • adding/removing APIs.
  • adding/removing properties.
  • adding/removing API-version.
  • adding a new service in Azure.

Failure to comply may result in delays for manifest application. Note this does not apply to data plane APIs.

  • If you are blocked on ARM review and want to get the PR merged urgently, please get the ARM oncall for reviews (RP Manifest Approvers team under Azure Resource Manager service) from IcM and reach out to them.
    Please follow the link to find more details on API review process.

Amogh Natu and others added 2 commits February 18, 2020 09:05
Added the missing 'description' field in CreatedBy element
@azure-pipelines

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

@openapi-sdkautomation

openapi-sdkautomation Bot commented Feb 19, 2020

Copy link
Copy Markdown

azure-sdk-for-python - Release

️✔️ succeeded [Logs] [Expand Details]
  • ️✔️ Generate from f953e95 with merge commit 8a8166f. SDK Automation 13.0.17.20200326.3
  • ️✔️azure-mgmt-databricks [Logs]  [Release SDK Changes]
    [build_package] /usr/lib/python3.6/distutils/dist.py:261: UserWarning: Unknown distribution option: 'long_description_content_type'
    [build_package]   warnings.warn(msg)
    [build_package] /usr/lib/python3.6/distutils/dist.py:261: UserWarning: Unknown distribution option: 'long_description_content_type'
    [build_package]   warnings.warn(msg)
    [breaking_change_setup] Ignoring mock: markers 'python_version <= "2.7"' don't match your environment
    [breaking_change_setup] Cannot uninstall requirement azure-nspkg, not installed
    [breaking_change_setup] Command '['/usr/local/bin/python', '-m', 'pip', 'uninstall', '-y', 'azure-nspkg']' returned non-zero exit status 1.
    [ChangeLog] Size of delta 39.660% size of original (original: 10416 chars, delta: 4131 chars)
    [ChangeLog] **Features**
    [ChangeLog] 
    [ChangeLog]   - Model Workspace has a new parameter workspace_id
    [ChangeLog]   - Model Workspace has a new parameter created_date_time
    [ChangeLog]   - Model Workspace has a new parameter updated_by
    [ChangeLog]   - Model Workspace has a new parameter workspace_url
    [ChangeLog]   - Model Workspace has a new parameter created_by

@openapi-sdkautomation

openapi-sdkautomation Bot commented Feb 19, 2020

Copy link
Copy Markdown

azure-sdk-for-go - Release

️✔️ succeeded [Logs] [Expand Details]

@openapi-sdkautomation

openapi-sdkautomation Bot commented Feb 19, 2020

Copy link
Copy Markdown

azure-sdk-for-java - Release

No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured

@openapi-sdkautomation

openapi-sdkautomation Bot commented Feb 19, 2020

Copy link
Copy Markdown

azure-sdk-for-net - Release

No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured

@azure-pipelines

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

1 similar comment
@azure-pipelines

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

@amoghnatu amoghnatu requested a review from achandmsft February 19, 2020 00:07
@openapi-sdkautomation

openapi-sdkautomation Bot commented Feb 19, 2020

Copy link
Copy Markdown

azure-sdk-for-js - Release

️✔️ succeeded [Logs] [Expand Details]
  • ️✔️ Generate from f953e95 with merge commit 8a8166f. SDK Automation 13.0.17.20200326.3
  • ️✔️@azure/arm-databricks [Logs]  [Release SDK Changes]
    [npmPack] npm WARN lifecycle @azure/arm-databricks@1.1.0~prepack: cannot run in wd @azure/arm-databricks@1.1.0 npm install && npm run build (wd=/z/work/azure-sdk-for-js/sdk/databricks/arm-databricks)

@amoghnatu

Copy link
Copy Markdown
Contributor Author

WaitForARMFeedback
Unable to add the label "WaitForARMFeedback" so addint it as a comment

@yungezz yungezz added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Feb 19, 2020
@amoghnatu amoghnatu requested a review from yungezz February 19, 2020 00:42
added readonly attribute to properties
@azure-pipelines

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

@ravrams

ravrams commented Feb 19, 2020

Copy link
Copy Markdown

Can you please update with below changes as discussed offline.

  1. New property 'properties.parameters.enableFedRampCertified'
  2. Returning 'properties.CreatedDateTime'
  3. VNet peering in the operation list
  4. AML workspace id in the example should be a resource uri
  5. Sku name value should be allowed value (in the example)
  6. Revert changes to remove properties
  7. Update examples as well.

Fixed all the below review comments.
1. New property 'properties.parameters.enableFedRampCertified'
2. Returning 'properties.CreatedDateTime'
3. VNet peering in the operation list
4. AML workspace id in the example should be a resource uri
5. Sku name value should be allowed value (in the example)
6. Revert changes to remove properties
7. Update examples as well.
@azure-pipelines

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

@amoghnatu

Copy link
Copy Markdown
Contributor Author

@yungezz can you confirm when we can merge this PR?

Fixed model validation errors.
"description": "Specifies the date and time when the workspace is created.",
"$ref": "#/definitions/CreatedDateTime"
},
"workspaceId": {

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.

adding properties to existing version is breaking change. need to do it in new version

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@yungezz We're going to backfill all existing databricks workspaces with these 2 properties. Any new workspaces created with this api-version will get these 2 properties by default. And all existing workspaces, we're going to backfill them next week. So this shouldn't technically remain as a breaking change after the backfilling is done. Also, since the backfill process is scheduled for next week, we request you to put a "DoNotMerge" label until we complete the process.

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.

HI @amoghnatu, thanks for sharing this. Adding new properties in response will break the GET-PUT pipeline, here's guideline of breaking change https://github.com/Azure/adx-documentation-pr/wiki/Breaking-changes-guidelines#new-property-added-to-response

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@yungezz Thanks for mentioning this. However, in our case, these 2 new properties are readonly and the customer will not be able to set them. Also, these are not available in the template so user cannot set them to provide as input. I have updated the spec to include readonly=true for these 2 properties. Kindly check.

@yungezz

yungezz commented Mar 6, 2020

Copy link
Copy Markdown
Member

since there're new properties added after ARM signoff. need ARM review again. Wait for ARM review.

@yungezz yungezz removed the DoNotMerge <valid label in PR review process> use to hold merge after approval label Mar 6, 2020
@azure-pipelines

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

@akning-ms

Copy link
Copy Markdown
Contributor

Hi @amoghnatu, Looks there is some change for package-lock.json file in your PR which is not supposed in swagger PR. can you remove this file change in this PR? Thanks

@azure-pipelines

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

@amoghnatu

Copy link
Copy Markdown
Contributor Author

@akning-ms Thanks for pointing it out. I have removed the file.

@azure-pipelines

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

@azure-pipelines

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

@ravbhatnagar

Copy link
Copy Markdown
Contributor

signing off from ARM side.

@ravbhatnagar ravbhatnagar added 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 Mar 12, 2020
@yungezz

yungezz commented Mar 12, 2020

Copy link
Copy Markdown
Member

hi @amoghnatu, could you pls fix CI failure, then the PR ok to merge.

@azure-pipelines

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

@openapi-sdkautomation

openapi-sdkautomation Bot commented Mar 16, 2020

Copy link
Copy Markdown

azure-cli-extensions - Release

No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured

@yungezz

yungezz commented Mar 19, 2020

Copy link
Copy Markdown
Member

hi @amoghnatu could you pls fix CI?

@yungezz

yungezz commented Mar 26, 2020

Copy link
Copy Markdown
Member

ping @amoghnatu

@yungezz yungezz closed this Apr 6, 2020
@yungezz yungezz reopened this Apr 6, 2020
@azure-pipelines

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

@yungezz yungezz added the Approved-BreakingChange DO NOT USE! OBSOLETE label. See https://github.com/Azure/azure-sdk-tools/issues/6374 label Apr 7, 2020
@yungezz yungezz merged commit 8a8166f into Azure:master Apr 7, 2020
00Kai0 pushed a commit to 00Kai0/azure-rest-api-specs that referenced this pull request Oct 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Approved-BreakingChange DO NOT USE! OBSOLETE label. See https://github.com/Azure/azure-sdk-tools/issues/6374 ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review potential-sdk-breaking-change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants