-
Notifications
You must be signed in to change notification settings - Fork 906
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
Add restore from S3 url #25216
Add restore from S3 url #25216
Conversation
barbaravaldez
commented
Jan 16, 2024
•
edited
Loading
edited
Pull Request Test Coverage Report for Build 7789684642Warning: This coverage report may be inaccurate.We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
💛 - Coveralls |
/** | ||
* Create a new credential | ||
* @param connectionUri The URI of the server connection. | ||
* @param credentialInfo |
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.
Needs a description here
const credentialInfo: azdata.CredentialInfo = { | ||
secret: `${result.accessKey}:${result.secretKey}`, | ||
url: result.s3Url.toString(), | ||
identity: 'S3 Access Key', |
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.
Would suggest linking to the docs about this hardcoded value
extensions/mssql/src/objectManagement/ui/restoreDatabaseDialog.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: Cory Rivera <corivera@microsoft.com>
On average, how long does it take to do the restore operation? Seems like the dialog is spinning for a while before succeeding. Before I noticed the spinner it almost seemed like the dialog wasn't doing anything at all. |
Adding the credential is very fast, what takes longer is to retrieve the restore information. I think this happens for the URL from blob restore as well. |
id: undefined | ||
} | ||
await this.objectManagementService.createCredential(this.connectionUri, this.credentialInfo); | ||
this.result.backupFilePath = `s3://${this.bucketDropdown.value}.s3.${this.regionInputBox.value}.amazonaws.com/${this.backupFilesDropdown.value}`; |
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.
Do any of these values need to be escaped?
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.
Approved pending my other comments