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

Managed Instance GeoRestore Cmds #4947

Merged
merged 4 commits into from
Jan 22, 2019
Merged

Managed Instance GeoRestore Cmds #4947

merged 4 commits into from
Jan 22, 2019

Conversation

lixiachena
Copy link
Contributor

New Cmdlets for Management.Sql to allow customers to get recoverable managed databases and create database by recovery

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.

New Cmdlets for Management.Sql to allow customers to get recoverable managed databases and create database by recovery
@openapi-portal-comment
Copy link

If you're a MSFT employee, click this link
to view this PR's validation status on our new OpenAPI Hub spec management tool.

@AutorestCI
Copy link

AutorestCI commented Dec 19, 2018

Automation for azure-sdk-for-ruby

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-ruby#2143

@AutorestCI
Copy link

AutorestCI commented Dec 19, 2018

Automation for azure-sdk-for-python

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

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@AutorestCI
Copy link

AutorestCI commented Dec 19, 2018

Automation for azure-sdk-for-js

A PR has been created for you:
Azure/azure-sdk-for-js#964

@AutorestCI
Copy link

AutorestCI commented Dec 19, 2018

Automation for azure-sdk-for-go

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

@AutorestCI
Copy link

AutorestCI commented Dec 19, 2018

Automation for azure-sdk-for-java

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

@AutorestCI
Copy link

AutorestCI commented Dec 19, 2018

Automation for azure-sdk-for-node

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

@hovsepm
Copy link
Contributor

hovsepm commented Dec 20, 2018

@lixiachena please add your spec to readme.md file as well.

@lixiachena
Copy link
Contributor Author

This feature is not available in public, I will add spec to readme.md once the feature is available in public.

@hovsepm hovsepm added WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required DoNotMerge <valid label in PR review process> use to hold merge after approval labels Jan 3, 2019
@hovsepm
Copy link
Contributor

hovsepm commented Jan 3, 2019

@lixiachena please fix model validation errors:

error : 
operationId: RecoverableManagedDatabases_ListByInstance
scenario: List recoverable databases by managed instances
source: response
responseCode: '200'
severity: 0
errorCode: OBJECT_ADDITIONAL_PROPERTIES
errorDetails:
  code: OBJECT_ADDITIONAL_PROPERTIES
  params:
    - - location
  message: 'Additional properties not allowed: location'
  path: value/0
  title: /definitions/RecoverableManagedDatabase
  description: A recoverable managed database resource.
  similarPaths:
    - value/1
  position:
    line: 170
    column: 35
  url: >-
    specification/sql/resource-manager/Microsoft.Sql/preview/2017-10-01-preview/recoverableManagedDatabases.json
  directives: {}
  jsonPosition:
    line: 12
    column: 19
  jsonUrl: >-
    specification/sql/resource-manager/Microsoft.Sql/preview/2017-10-01-preview/examples/ListRecoverableManagedDatabasesByServer.json
 error : 
operationId: RecoverableManagedDatabases_Get
scenario: Gets a recoverable databases by managed instances
source: request
responseCode: ALL
severity: 0
errorCode: REQUIRED_PARAMETER_EXAMPLE_NOT_FOUND
errorDetails:
  code: REQUIRED_PARAMETER_EXAMPLE_NOT_FOUND
  id: OAV105
  message: >-
    In operation "RecoverableManagedDatabases_Get", parameter
    recoverableDatabaseName is required in the swagger spec but is not present
    in the provided example parameter values.
 error : 
operationId: RecoverableManagedDatabases_Get
scenario: Gets a recoverable databases by managed instances
source: response
responseCode: '200'
severity: 0
errorCode: OBJECT_ADDITIONAL_PROPERTIES
errorDetails:
  code: OBJECT_ADDITIONAL_PROPERTIES
  params:
    - - location
  message: 'Additional properties not allowed: location'
  path: ''
  title: /definitions/RecoverableManagedDatabase
  description: A recoverable managed database resource.
  position:
    line: 170
    column: 35
  url: >-
    specification/sql/resource-manager/Microsoft.Sql/preview/2017-10-01-preview/recoverableManagedDatabases.json
  directives: {}
  jsonUrl: >-
    specification/sql/resource-manager/Microsoft.Sql/preview/2017-10-01-preview/examples/GetRecoverableManagedDatabase.json

Modify example files and readme.md for get recoverable managed databases
Copy link
Contributor

@ravbhatnagar ravbhatnagar left a comment

Choose a reason for hiding this comment

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

nothing major, couple of minor notes.

"description": "The recoverable managed database's properties.",
"type": "object",
"properties": {
"lastAvailableBackupDate": {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be ISO 8601 compliant and so the format should be date-time

Copy link
Contributor Author

@lixiachena lixiachena Jan 11, 2019

Choose a reason for hiding this comment

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

This is actually a string format. This value will be used later by customer for restore

Copy link
Contributor

Choose a reason for hiding this comment

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

Lixia, what should this property's data type be in generated C# code? string, or datetime?

"produces": [
"application/json"
],
"paths": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to delete a recoverableDatabase through an API? Or is this a valid scenario? I am assuming that databases are automatically categorized as recoverable or not by the service.

Copy link
Contributor Author

@lixiachena lixiachena Jan 11, 2019

Choose a reason for hiding this comment

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

Recoverabledatabase is a read only. Customer can't drop it

Copy link
Contributor

Choose a reason for hiding this comment

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

@ravbhatnagar their lifetime is managed by the service, so no create or delete API is possible or needed.

@ravbhatnagar ravbhatnagar added the ARMChangesRequested <valid label in PR review process>add this label when require changes after ARM review label Jan 9, 2019
@hovsepm hovsepm removed their assignment Jan 10, 2019
@ravbhatnagar
Copy link
Contributor

@lixiachena - please take a look at the comments. thanks!

@lixiachena
Copy link
Contributor Author

@lixiachena please add your spec to readme.md file as well.
Added. Thanks

@lixiachena lixiachena closed this Jan 11, 2019
@hovsepm hovsepm removed their request for review January 14, 2019 21:37
@lixiachena lixiachena reopened this Jan 14, 2019
@openapi-portal-comment
Copy link

If you're a MSFT employee, click this link
to view this PR's validation status on our new OpenAPI Hub spec management tool.

@hovsepm
Copy link
Contributor

hovsepm commented Jan 14, 2019

@ravbhatnagar per PR author your feedback is addressed. Could you verify?

Copy link
Contributor

@KrisBash KrisBash left a comment

Choose a reason for hiding this comment

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

Signing off for ARM

@KrisBash KrisBash added ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review and removed ARMChangesRequested <valid label in PR review process>add this label when require changes after ARM review WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Jan 17, 2019
@sarangan12
Copy link
Member

Left feedback to fix the model validator error (through IM), Waiting for response

Update recoverableManagedDatabaseName to recoverableDatabaseName
@sarangan12 sarangan12 merged commit 9899774 into Azure:master Jan 22, 2019
lixiachena added a commit to lixiachena/azure-sdk-for-net that referenced this pull request Jan 29, 2019
Swagger PR Links
Azure/azure-rest-api-specs#4947

Add SDK for database recovery on Managed Instance

•Add getrecoverabledatabases on managed instance and create databases using geo-backup

•Add test

•Generate client from azure and update session records

•Add release notes
dsgouda pushed a commit to Azure/azure-sdk-for-net that referenced this pull request Jan 29, 2019
Swagger PR Links
Azure/azure-rest-api-specs#4947

Add SDK for database recovery on Managed Instance

•Add getrecoverabledatabases on managed instance and create databases using geo-backup

•Add test

•Generate client from azure and update session records

•Add release notes
TalluriAnusha pushed a commit to AsrOneSdk/azure-rest-api-specs that referenced this pull request Feb 6, 2019
* Managed Instance GeoRestore Cmds

New Cmdlets for Management.Sql to allow customers to get recoverable managed databases and create database by recovery

* Modify example files and readme.md

Modify example files and readme.md for get recoverable managed databases

* Update GetRecoverableManagedDatabase.json

Fix one typo

* Update recoverableManagedDatabaseName to recoverableDatabaseName

Update recoverableManagedDatabaseName to recoverableDatabaseName
mentat9 pushed a commit to mentat9/azure-sdk-for-net that referenced this pull request Jun 10, 2019
Swagger PR Links
Azure/azure-rest-api-specs#4947

Add SDK for database recovery on Managed Instance

•Add getrecoverabledatabases on managed instance and create databases using geo-backup

•Add test

•Generate client from azure and update session records

•Add release notes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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 review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants