Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[READY FOR DESIGN REVIEW] -> MachineLearningServices: Adding new API version and definition for AmlInstance compute type #7323

Closed
wants to merge 36 commits into from
Closed

[READY FOR DESIGN REVIEW] -> MachineLearningServices: Adding new API version and definition for AmlInstance compute type #7323

wants to merge 36 commits into from

Conversation

vhamine
Copy link

@vhamine vhamine commented Sep 25, 2019 •
edited
Loading

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.

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Sep 25, 2019 •
edited
Loading

In Testing, Please Ignore

[Logs] (Generated from 9a2ab25, Iteration 36)

Failed .NET: test-repo-billy/azure-sdk-for-net [Logs]
  • No packages generated.
Failed Go: test-repo-billy/azure-sdk-for-go [Logs]
  • No packages generated.
Failed Python: test-repo-billy/azure-sdk-for-python [Logs]
  • No packages generated.
Failed JavaScript: test-repo-billy/azure-sdk-for-js [Logs]
  • No packages generated.

@AutorestCI
Copy link

AutorestCI commented Sep 25, 2019 •
edited
Loading

Automation for azure-sdk-for-python

A PR has been created for you based on this PR content.

Once this PR will be merged, content will be added to your service PR:
Azure/azure-sdk-for-python#7737

@AutorestCI
Copy link

AutorestCI commented Sep 25, 2019 •
edited
Loading

Automation for azure-sdk-for-go

A PR has been created for you:
Azure/azure-sdk-for-go#6171

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

This was referenced Sep 26, 2019
Closed
Closed
@Juliehzl
Copy link
Contributor

Hi @vhamine, why do you send a PR in draft? Are you ready for review process?

@Juliehzl Juliehzl added the DoNotMerge <valid label in PR review process> use to hold merge after approval label Sep 26, 2019
@vhamine
Copy link
Author

vhamine commented Sep 26, 2019

Hi @vhamine, why do you send a PR in draft? Are you ready for review process?

@vhamine vhamine closed this Sep 26, 2019
@vhamine vhamine reopened this Sep 26, 2019
@vhamine
Copy link
Author

vhamine commented Sep 26, 2019

Hi @vhamine, why do you send a PR in draft? Are you ready for review process?

The PR is not ready for review yet as I am still making changes. This being my first interaction with azure rest API specification, I thought I should create a draft PR and get familiar with the pipeline checks. I plan to attend the office hours today and send a PR for review by EOD. Thanks!

@vhamine
Internal review feedback: Adding sshPort to sshSettings and separatin…
bc06bc8 
…g applicationURIs from connectivityEndpoints
@AutorestCI AutorestCI mentioned this pull request Oct 9, 2019
Closed
@AutorestCI AutorestCI mentioned this pull request Oct 10, 2019
Closed
@sanjaiganesh sanjaiganesh 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 Oct 11, 2019
@vhamine
Copy link
Author

vhamine commented Oct 11, 2019

@Juliehzl this PR was signed-off by ARM. We do not want to merge this PR yet since our back-end is still being implemented. We should co-ordinate merging this before Ignite 2019. Can you please let us know what the next steps should be?

CC: @ahcan76

@Juliehzl
Copy link
Contributor

Hi @vhamine, I have a look at your swagger specification. It looks good to me and I already approved for your PR. All review process is completed before clicking the merge button.
If you don't want it to be merged now, I will add DONOTMERGE label for your PR and when you are ready to merge, just let me know and I will click the merge button for you.

@Juliehzl Juliehzl added the DoNotMerge <valid label in PR review process> use to hold merge after approval label Oct 17, 2019
@ramagott
Updating the ComputeInstance state list
2678804 
1. Updating the ComputeInstance state list
2. Changing the return type for CI start, stop, and restart operations
@openapi-sdkautomation openapi-sdkautomation bot mentioned this pull request Oct 28, 2019
Open
@AutorestCI AutorestCI mentioned this pull request Oct 29, 2019
Closed
@ramagott ramagott mentioned this pull request Feb 21, 2020
Merged
5 tasks
@akning-ms
Copy link
Contributor

Hi @vhamine, Can we know what is this PR status as it is existing for a long time？

@ramagott
Copy link
Member

Hi @vhamine, Can we know what is this PR status as it is existing for a long time？

This PR is not required any more. Can you close it?

@ghost
Copy link

ghost commented Dec 27, 2020

Hi, @vhamine. Your PR has no update for 14 days and it is marked as stale PR. If no further update for over 14 days, the bot will close the PR. If you want to refresh the PR, please remove no-recent-activity label.

@ghost ghost added the no-recent-activity label Dec 27, 2020
@Juliehzl Juliehzl closed this Feb 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sanjaiganesh sanjaiganesh sanjaiganesh left review comments

@cyly cyly cyly left review comments

@Juliehzl Juliehzl Juliehzl approved these changes

Assignees

@Juliehzl Juliehzl

Labels
ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review DoNotMerge <valid label in PR review process> use to hold merge after approval
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@vhamine @AutorestCI @azuresdkci @Juliehzl @akning-ms @ramagott @sanjaiganesh @cyly