-
Notifications
You must be signed in to change notification settings - Fork 5.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
AzureML 2020-09-01-preview codejobs #10421
Conversation
… preview/2020-05-15-preview to version 2020-09-01-preview
[Staging] Swagger Validation Report
️✔️ |
Azure Pipelines successfully started running 1 pipeline(s). |
Azure CLI Extension Generation - Release
|
azure-sdk-for-go - Release
|
azure-sdk-for-net - Release
|
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
|
azure-sdk-for-js - Release
|
azure-sdk-for-python - Release
- Breaking Change detected in SDK
|
azure-sdk-for-python-track2 - 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
|
Trenton Generation - 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
|
Can one of the admins verify this patch? |
The Lint error is saying there is a missing list API for LinkedWorkspace, but there is an API for this: Line 1246 in c740a55
This file is also a copy from the previous version (no changes have been made to this file). |
Our team (AzureML) also has another PR for a separate set of APIs: #10420. These PRs should be able to be merged in any order (one of us will handle any conflicts). |
Azure Pipelines successfully started running 1 pipeline(s). |
...ices/resource-manager/Microsoft.MachineLearningServices/preview/2020-09-01-preview/jobs.json
Show resolved
Hide resolved
"update" | ||
] | ||
}, | ||
"tags": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tags [](start = 9, length = 4)
Tags are part of top level ARM object. Do you need separate tagging in resource properties?
All the Governance scenario like policy uses top level tags.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use separate tagging in our MachineLearning objects (we have several scenarios that use them), and we can't remove them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we rename these to specific tags something like MLTags? This will cause confusion otherwise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a proxy resource - I'm not seeing tags on the top level ARM proxy resource entity. Unfortunately, we can't rename these, since this naming is already used across our product.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tags has a special purpose in Azure control plane services; customer understands tags have special meaning and this needs to be uphold. Please start and email thread and explain the reasoning of why the interface can not be adapted.
...ices/resource-manager/Microsoft.MachineLearningServices/preview/2020-09-01-preview/jobs.json
Show resolved
Hide resolved
Azure Pipelines successfully started running 1 pipeline(s). |
@ArcturusZhang I made the changes you requested. Thanks! |
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry but I made a mistake in the statement of nextlink. Could you please have a look?
Other than this, this LGTM
...er/Microsoft.MachineLearningServices/preview/2020-09-01-preview/machineLearningServices.json
Outdated
Show resolved
Hide resolved
Azure Pipelines successfully started running 1 pipeline(s). |
azure-resource-manager-schemas - Release
|
...ices/resource-manager/Microsoft.MachineLearningServices/preview/2020-09-01-preview/jobs.json
Show resolved
Hide resolved
...ices/resource-manager/Microsoft.MachineLearningServices/preview/2020-09-01-preview/jobs.json
Show resolved
Hide resolved
Added some minor comments on the PR, not blocking the PR from ARM side. Please follow the process given @ https://dev.azure.com/azure-sdk/internal/_wiki/wikis/internal.wiki/202/Overall-Process-of-AME-Onboarding |
@ArcturusZhang Thanks for the review! I made the requested changes - can you sign off if it looks good to you? |
@j-so seems ARM still left some questions, could please have them resolved also? |
Azure Pipelines successfully started running 1 pipeline(s). |
They have been resolved. Thanks |
* Adds base for updating Microsoft.MachineLearningServices from version preview/2020-05-15-preview to version 2020-09-01-preview * Updates readme * Updates API version in new specs and examples * add new codejob files * update readme with new json * update readme * use common shared type * prettier * move away from shared common type * fix machinelearningservices * use common type and change naming of codejobs * comments * fix examples * prettier * removve required nextlink * make status readonly and add required
This reverts commit 73926a4.
MSFT employees can try out our new experience at OpenAPI Hub - one location for using our validation tools and finding your workflow.
Contribution checklist:
If any further question about AME onboarding or validation tools, please view the FAQ.
ARM API Review Checklist
Ensure to check this box if one of the following scenarios meet updates in the PR, so that label “WaitForARMFeedback” will be added automatically to involve ARM API Review. Failure to comply may result in delays for manifest application. Note this does not apply to data plane APIs, all “removals” and “adding a new property” no more require ARM API review.
If you are blocked on ARM review and want to get the PR merged with urgency, please get the ARM oncall for reviews (RP Manifest Approvers team under Azure Resource Manager service) from IcM and reach out to them.
Breaking Change Review Checklist
If there are following updates in the PR, ensure to request an approval from API Review Board as defined in the Breaking Change Policy.
Please follow the link to find more details on PR review process.