-
Notifications
You must be signed in to change notification settings - Fork 150
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
generic way to support scriptable operations #438
Changes from 6 commits
b90d297
945ad89
86ba71d
2b6af04
751764c
24baa6b
21b3793
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,7 +52,7 @@ public class BackupOperation : IBackupOperation | |
/// this is used when the backup dialog is launched in the context of a backup device | ||
/// The InitialBackupDestination will be loaded in LoadData | ||
private string initialBackupDestination = string.Empty; | ||
|
||
// Helps in populating the properties of an Azure blob given its URI | ||
private class BlobProperties | ||
{ | ||
|
@@ -163,6 +163,16 @@ public string ScriptContent | |
} | ||
} | ||
|
||
public string ErrorMessage | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor: please add doc comments to public properties #Resolved |
||
{ | ||
get | ||
{ | ||
return string.Empty; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what's the point of this property? #WontFix There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added the error message to IBackupOperation so that the task manager get the error of the operation but didn't integrate the new changes with backup In reply to: 134057582 [](ancestors = 134057582) |
||
} | ||
} | ||
|
||
public SqlTask SqlTask { get; set; } | ||
|
||
/// <summary> | ||
/// Execute backup | ||
/// </summary> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ | |
// | ||
|
||
using System.Collections.Generic; | ||
using Microsoft.SqlTools.ServiceLayer.TaskServices; | ||
using Microsoft.SqlTools.ServiceLayer.Utility; | ||
using Newtonsoft.Json.Linq; | ||
|
||
|
@@ -12,7 +13,7 @@ namespace Microsoft.SqlTools.ServiceLayer.DisasterRecovery.Contracts | |
/// <summary> | ||
/// Restore request parameters | ||
/// </summary> | ||
public class RestoreParams : GeneralRequestDetails | ||
public class RestoreParams : GeneralRequestDetails, IScriptableRequestParams | ||
{ | ||
/// <summary> | ||
/// Restore session id. The parameter is optional and if passed, an existing plan will be used | ||
|
@@ -140,6 +141,26 @@ internal IEnumerable<string> SelectedBackupSets | |
SetOptionValue(RestoreOptionsHelper.SelectedBackupSets, value); | ||
} | ||
} | ||
|
||
/// <summary> | ||
/// The executation mode for the operation. default is execution | ||
/// </summary> | ||
public TaskExecutionMode TaskExecutionMode { get; set; } | ||
|
||
/// <summary> | ||
/// Target Database name | ||
/// </summary> | ||
public string DatabaseName | ||
{ | ||
get | ||
{ | ||
return TargetDatabaseName; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do we have a property that gets & sets another property? Should we remove this property or the other one? #WontFix There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added the DatabaseName to the request base class assuming all requests will have a database name field. TargetDatabase is the same as DatabaseName if restore so that's why I'm implementing it this way In reply to: 134058351 [](ancestors = 134058351) |
||
} | ||
set | ||
{ | ||
TargetDatabaseName = value; | ||
} | ||
} | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -224,15 +224,10 @@ internal async Task HandleRestoreRequest( | |
if (restoreDataObject != null) | ||
{ | ||
// create task metadata | ||
TaskMetadata metadata = new TaskMetadata(); | ||
metadata.ServerName = connInfo.ConnectionDetails.ServerName; | ||
metadata.DatabaseName = restoreParams.TargetDatabaseName; | ||
metadata.Name = SR.RestoreTaskName; | ||
metadata.IsCancelable = true; | ||
metadata.Data = restoreDataObject; | ||
TaskMetadata metadata = TaskMetadata.Create(restoreParams, SR.RestoreTaskName, restoreDataObject, ConnectionServiceInstance); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens to IsCancelable? Is the default True and you just override or don't? #Pending There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I removed it from here and if the cancel function is defined I assume it's cancelable In reply to: 134042668 [](ancestors = 134042668) |
||
|
||
// create restore task and perform | ||
SqlTask sqlTask = SqlTaskManagerInstance.CreateAndRun(metadata, this.restoreDatabaseService.RestoreTaskAsync, restoreDatabaseService.CancelTaskAsync); | ||
SqlTask sqlTask = SqlTaskManagerInstance.CreateAndRun<SqlTask>(metadata); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how are we setting restore's CancelTask in SqlTask? |
||
response.TaskId = sqlTask.TaskId.ToString(); | ||
} | ||
else | ||
|
@@ -285,8 +280,7 @@ internal async Task HandleBackupRequest( | |
TaskMetadata metadata = new TaskMetadata(); | ||
metadata.ServerName = connInfo.ConnectionDetails.ServerName; | ||
metadata.DatabaseName = connInfo.ConnectionDetails.DatabaseName; | ||
metadata.Data = backupOperation; | ||
metadata.IsCancelable = true; | ||
metadata.TaskOperation = backupOperation; | ||
|
||
if (backupParams.IsScripting) | ||
{ | ||
|
@@ -414,7 +408,7 @@ internal void ScriptBackup(BackupOperation backupOperation) | |
/// <returns></returns> | ||
internal async Task<TaskResult> PerformBackupTaskAsync(SqlTask sqlTask) | ||
{ | ||
IBackupOperation backupOperation = sqlTask.TaskMetadata.Data as IBackupOperation; | ||
IBackupOperation backupOperation = sqlTask.TaskMetadata.TaskOperation as IBackupOperation; | ||
TaskResult result = new TaskResult(); | ||
|
||
// Create a task to perform backup | ||
|
@@ -463,7 +457,7 @@ await Task.Factory.StartNew(() => | |
/// <returns></returns> | ||
internal async Task<TaskResult> CancelBackupTaskAsync(SqlTask sqlTask) | ||
{ | ||
IBackupOperation backupOperation = sqlTask.TaskMetadata.Data as IBackupOperation; | ||
IBackupOperation backupOperation = sqlTask.TaskMetadata.TaskOperation as IBackupOperation; | ||
TaskResult result = new TaskResult(); | ||
|
||
await Task.Factory.StartNew(() => | ||
|
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.
Nice catch :) #WontFix