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.

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

Added HANA instance restart API #3761

Merged
merged 3 commits into from
Aug 31, 2018
Merged

Added HANA instance restart API #3761

merged 3 commits into from
Aug 31, 2018

Conversation

PakDLiu
Copy link
Contributor

@PakDLiu PakDLiu commented Aug 29, 2018

This checklist is used to make sure that common issues in a pull request are addressed. This will expedite the process of getting your pull request merged and avoid extra work on your part to fix issues discovered during the review process.

PR information

  • The title of the PR is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For information on cleaning up the commits in your pull request, see this page.
  • Except for special cases involving multiple contributors, the PR is started from a fork of the main repository, not a branch.
  • If applicable, the PR references the bug/issue that it fixes.
  • Swagger files are correctly named (e.g. the api-version in the path should match the api-version in the spec).

Quality of Swagger

I understand that this is meant to be an async API and should be using the x-ms-long-running-operation and the x-ms-long-running-operation-options tags. However, our team is trying to demo this restart functionality next month (ran by us, not the customer) and we didn't have enough time to implement the async operations API. The intent of the demo is to execute this API (Either via the portal or CLI), and we will continuously call GET till it returns the correct state. This spec will be updated with the correct long-running tags and with operation APIs before we go public. My lead is aware of all of this and has approved it. Thank you for your understanding.

@AutorestCI
Copy link

AutorestCI commented Aug 29, 2018

Automation for azure-sdk-for-python

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-python#3251

@AutorestCI
Copy link

AutorestCI commented Aug 29, 2018

Automation for azure-sdk-for-ruby

Nothing to generate for azure-sdk-for-ruby

@AutorestCI
Copy link

AutorestCI commented Aug 29, 2018

Automation for azure-sdk-for-node

Nothing to generate for azure-sdk-for-node

@AutorestCI
Copy link

AutorestCI commented Aug 29, 2018

Automation for azure-sdk-for-java

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-java#2297

@AutorestCI
Copy link

AutorestCI commented Aug 29, 2018

Automation for azure-sdk-for-go

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-go#2605

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

Copy link
Contributor

@lagalbra lagalbra left a comment

Choose a reason for hiding this comment

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

Other than examples, which I think should definitely be added, LGTM

],
"responses": {
"202": {
"description": "Accepted"
Copy link
Member

Choose a reason for hiding this comment

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

WHat will happen if the restart fails?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It will return a 404 if the instance is not found, 400 if the instance is not in a valid power state (ie: already restarting) and 500 if something on the server went wrong.

I noticed in the compute VM spec only has 200 and 202, do you not have to set all the return codes? or only the valid ones? Same on the GET VM, only 200.

@sarangan12 sarangan12 merged commit e085517 into Azure:master Aug 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants