-
Notifications
You must be signed in to change notification settings - Fork 744
feat(amazonq): display transformation history and add ability to resume interrupted jobs #7781
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
Changes from all commits
ab1d404
c86fa10
941d18e
0bb9102
51d749a
deadc6d
31bd370
caa800d
eaa61b9
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 |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| { | ||
| "type": "Feature", | ||
| "description": "/transform: Show transformation history in Transformation Hub and allow users to resume jobs" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,7 @@ import { | |
| TransformationType, | ||
| TransformationCandidateProject, | ||
| RegionProfile, | ||
| sessionJobHistory, | ||
| } from '../models/model' | ||
| import { | ||
| createZipManifest, | ||
|
|
@@ -474,6 +475,30 @@ export async function startTransformationJob( | |
| codeTransformRunTimeLatency: calculateTotalLatency(transformStartTime), | ||
| }) | ||
| }) | ||
|
|
||
| // create local history folder(s) and store metadata | ||
| const jobHistoryPath = path.join(os.homedir(), '.aws', 'transform', transformByQState.getProjectName(), jobId) | ||
|
Contributor
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. Is this the same folder as where the QCT CLI stores artifacts?
Contributor
Author
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. No, the CLI stores artifacts in |
||
| if (!fs.existsSync(jobHistoryPath)) { | ||
| fs.mkdirSync(jobHistoryPath, { recursive: true }) | ||
| } | ||
| transformByQState.setJobHistoryPath(jobHistoryPath) | ||
| // save a copy of the upload zip | ||
| fs.copyFileSync(transformByQState.getPayloadFilePath(), path.join(jobHistoryPath, 'zipped-code.zip')) | ||
|
|
||
| const fields = [ | ||
| jobId, | ||
| transformByQState.getTransformationType(), | ||
| transformByQState.getSourceJDKVersion(), | ||
| transformByQState.getTargetJDKVersion(), | ||
| transformByQState.getCustomDependencyVersionFilePath(), | ||
| transformByQState.getCustomBuildCommand(), | ||
| transformByQState.getTargetJavaHome(), | ||
| transformByQState.getProjectPath(), | ||
| transformByQState.getStartTime(), | ||
| ] | ||
|
|
||
| const jobDetails = fields.join('\t') | ||
| fs.writeFileSync(path.join(jobHistoryPath, 'metadata.txt'), jobDetails) | ||
| } catch (error) { | ||
| getLogger().error(`CodeTransformation: ${CodeWhispererConstants.failedToStartJobNotification}`, error) | ||
| const errorMessage = (error as Error).message.toLowerCase() | ||
|
|
@@ -724,9 +749,18 @@ export async function postTransformationJob() { | |
| }) | ||
| } | ||
|
|
||
| if (transformByQState.getPayloadFilePath()) { | ||
| // delete original upload ZIP at very end of transformation | ||
| fs.rmSync(transformByQState.getPayloadFilePath(), { force: true }) | ||
| // delete original upload ZIP at very end of transformation | ||
| fs.rmSync(transformByQState.getPayloadFilePath(), { force: true }) | ||
|
|
||
| if ( | ||
| transformByQState.isSucceeded() || | ||
| transformByQState.isPartiallySucceeded() || | ||
| transformByQState.isCancelled() | ||
|
Contributor
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. Instead of checking the job status to delete these files, just check if
Contributor
Author
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. The reason I was thinking of using status as the check here is because if the job fails or is incomplete on client side, we do not want to delete the metadata file and zip copy since those are needed to resume the job. Is there a better way to do this check? Sidenote: I will be moving the original zip deletion (lines 757-760) outside of this if clause since that can be deleted regardless |
||
| ) { | ||
| // delete the copy of the upload ZIP | ||
| fs.rmSync(path.join(transformByQState.getJobHistoryPath(), 'zipped-code.zip'), { force: true }) | ||
| // delete transformation job metadata file (no longer needed) | ||
| fs.rmSync(path.join(transformByQState.getJobHistoryPath(), 'metadata.txt'), { force: true }) | ||
| } | ||
| // delete temporary build logs file | ||
| const logFilePath = path.join(os.tmpdir(), 'build-logs.txt') | ||
|
|
@@ -739,31 +773,52 @@ export async function postTransformationJob() { | |
| if (transformByQState.isSucceeded() || transformByQState.isPartiallySucceeded()) { | ||
| await vscode.commands.executeCommand('aws.amazonq.transformationHub.reviewChanges.startReview') | ||
| } | ||
|
|
||
| // store job details and diff path locally (history) | ||
| // TODO: ideally when job is cancelled, should be stored as CANCELLED instead of FAILED (remove this if statement after bug is fixed) | ||
| if (!transformByQState.isCancelled()) { | ||
| const historyLogFilePath = path.join(os.homedir(), '.aws', 'transform', 'transformation_history.tsv') | ||
| // create transform folder if necessary | ||
| if (!fs.existsSync(historyLogFilePath)) { | ||
| fs.mkdirSync(path.dirname(historyLogFilePath), { recursive: true }) | ||
| // create headers of new transformation history file | ||
| fs.writeFileSync(historyLogFilePath, 'date\tproject_name\tstatus\tduration\tdiff_patch\tsummary\tjob_id\n') | ||
| } | ||
| const latest = sessionJobHistory[transformByQState.getJobId()] | ||
| const fields = [ | ||
| latest.startTime, | ||
| latest.projectName, | ||
| latest.status, | ||
| latest.duration, | ||
| transformByQState.isSucceeded() || transformByQState.isPartiallySucceeded() | ||
| ? path.join(transformByQState.getJobHistoryPath(), 'diff.patch') | ||
| : '', | ||
| transformByQState.isSucceeded() || transformByQState.isPartiallySucceeded() | ||
| ? path.join(transformByQState.getJobHistoryPath(), 'summary', 'summary.md') | ||
| : '', | ||
| transformByQState.getJobId(), | ||
| ] | ||
|
|
||
| const jobDetails = fields.join('\t') + '\n' | ||
| fs.writeFileSync(historyLogFilePath, jobDetails, { flag: 'a' }) // 'a' flag used to append to file | ||
| await vscode.commands.executeCommand( | ||
| 'aws.amazonq.transformationHub.updateContent', | ||
| 'job history', | ||
| undefined, | ||
| true | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| export async function transformationJobErrorHandler(error: any) { | ||
| if (!transformByQState.isCancelled()) { | ||
| // means some other error occurred; cancellation already handled by now with stopTransformByQ | ||
| await stopJob(transformByQState.getJobId()) | ||
| transformByQState.setToFailed() | ||
| transformByQState.setPolledJobStatus('FAILED') | ||
| // jobFailureErrorNotification should always be defined here | ||
| const displayedErrorMessage = | ||
| transformByQState.getJobFailureErrorNotification() ?? CodeWhispererConstants.failedToCompleteJobNotification | ||
|
Contributor
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. Can we just add some context to the PR why we are getting rid of these so reviewers are aware?
Contributor
Author
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. We decided to remove the |
||
| transformByQState.setJobFailureErrorChatMessage( | ||
| transformByQState.getJobFailureErrorChatMessage() ?? CodeWhispererConstants.failedToCompleteJobChatMessage | ||
| ) | ||
| void vscode.window | ||
| .showErrorMessage(displayedErrorMessage, CodeWhispererConstants.amazonQFeedbackText) | ||
| .then((choice) => { | ||
| if (choice === CodeWhispererConstants.amazonQFeedbackText) { | ||
| void submitFeedback( | ||
| placeholder, | ||
| CodeWhispererConstants.amazonQFeedbackKey, | ||
| getFeedbackCommentData() | ||
| ) | ||
| } | ||
| }) | ||
| } else { | ||
| transformByQState.setToCancelled() | ||
| transformByQState.setPolledJobStatus('CANCELLED') | ||
|
|
||
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.
Can we just explain this for context?
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.
Using
TransformationHubViewProvider.instancehere to construct the instance implements the singleton pattern and ensures that only one instance exists throughout. That way, whenTransformationHubViewProvider.instanceis referenced in different parts of the code, we get the same instance every time. I was having issues with accessing the instance with the way it was constructed previously since there seemed to be two instances that existed at the same time.