Skip to content

Conversation

@tgodara-aws
Copy link
Contributor

@tgodara-aws tgodara-aws commented Jul 28, 2025

Problem

Users cannot see previous job details (status, project name, job id, etc) and cannot access the final diff patch. Network issues can cause jobs to fail on client side while they continue on the backend, and users have no way to access those artifacts.

Solution

Repurpose the job status table to show most recent 10 jobs run in the last 30 days, including links to final diff patch and summary files. Allow users to retrieve missing artifacts for jobs and to resume incomplete jobs via refresh button.

History Table - New Text View History Button
  • Treat all work as PUBLIC. Private feature/x branches will not be squash-merged at release time.
  • Your code changes must meet the guidelines in CONTRIBUTING.md.
  • License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

// Convert milliseconds to days (1000ms * 60s * 60min * 24hr)
const daysDifference = timeDifference / (1000 * 60 * 60 * 24)

return daysDifference <= 30
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to explain why 30?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll just provide some context in this reply: we decided to only show users jobs from up to 30 days ago because on our backend we only store jobs and artifacts for up to 30 days. This way we align with that storage policy.

void setContext('gumby.wasQCodeTransformationUsed', false)

const transformationHubViewProvider = new TransformationHubViewProvider()
const transformationHubViewProvider = TransformationHubViewProvider.instance
Copy link
Contributor

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using TransformationHubViewProvider.instance here to construct the instance implements the singleton pattern and ensures that only one instance exists throughout. That way, when TransformationHubViewProvider.instance is 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.

transformByQState.setPolledJobStatus('FAILED')
// jobFailureErrorNotification should always be defined here
const displayedErrorMessage =
transformByQState.getJobFailureErrorNotification() ?? CodeWhispererConstants.failedToCompleteJobNotification
Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We decided to remove the stopJob() call because with the introduction of the refresh functionality, users can actually resume their jobs within 12 hours. This call was initially added so that jobs that were interrupted on client side would not just run till they time out on the backend. However, with this feature, since users can now continue their jobs, stopping the job on the backend would prevent users from taking this action.

: this.getTableMarkup(sessionJobHistory[transformByQState.getJobId()])
: this.getTableMarkup(jobsToDisplay)
}
<script>
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, are there other examples of us returning script tags within HTML?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are a few that I can see: showPlanProgress() method in TransformationHubViewProvider.ts, getWebviewContent() method in webViewPanel.ts -- this one uses it similarly to how I have done so here --, and getHtml() method in referenceLogViewProvider.ts

})

// create local history folder(s) and store metadata
const jobHistoryPath = path.join(os.homedir(), '.aws', 'transform', transformByQState.getProjectName(), jobId)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the same folder as where the QCT CLI stores artifacts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, the CLI stores artifacts in .aws/qcodetransform/ (specifically within the transformation_projects subdirectory)

if (
transformByQState.isSucceeded() ||
transformByQState.isPartiallySucceeded() ||
transformByQState.isCancelled()
Copy link
Contributor

Choose a reason for hiding this comment

The 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 fs.existsSync() to determine whether or not to delete the files

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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

? 'disabled title="A job is ongoing"'
: job.status === 'CANCELLED' ||
job.status === 'STOPPED' ||
job.status === 'FAILED_BE'
Copy link
Contributor

Choose a reason for hiding this comment

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

How is FAILED_BE managed? We already have FAILED so wondering if we need this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added FAILED_BE for the case where a job actually fails on the backend, though users will still only see it show up as FAILED in the Transformation Hub. In such a case we do not want users to be able to refresh that job because there are no updates for it on the backend. I had to add this local status because when checking for any jobs that may need to be resumed, I search for a status of FAILED in the local transformation history file (since this is the status that shows up on the frontend in the case of a client-side failure, like a network disruption). We don't want to mislead users into thinking an actual failed job is still refreshable or "in progress," so this FAILED_BE status helps differentiate that. But it's only used in the context of the history file/table.

Perhaps there is a better way I could've done this?

// artifacts should be available to download
jobHistoryPath = await this.retrieveArtifacts(jobId, projectName)

// delete metadata and zipped code files, if they exist
Copy link
Contributor

Choose a reason for hiding this comment

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

what happens if they don't exist?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nothing happens; since I'm using { force: true } here, no error is thrown even if the files don't exist

@tgodara-aws tgodara-aws force-pushed the feature/transformation-history-table branch from 8e20bee to 1609879 Compare July 31, 2025 04:43
@tgodara-aws tgodara-aws force-pushed the feature/transformation-history-table branch 2 times, most recently from d9ccc41 to ad8ef31 Compare August 1, 2025 20:50
@tgodara-aws tgodara-aws marked this pull request as ready for review August 1, 2025 21:15
@tgodara-aws tgodara-aws requested a review from a team as a code owner August 1, 2025 21:15
@tgodara-aws tgodara-aws force-pushed the feature/transformation-history-table branch from ad8ef31 to a50281b Compare August 1, 2025 23:48
laileni-aws
laileni-aws previously approved these changes Aug 2, 2025
…tches locally; write history details to local log file
…diff patch if available), also added isWithin30Days util func for future use
…build log; open diff patch and summary directly in VSCode
…less often, limit table to 10 rows and validate dates
… transform initiated; create unit tests for job history feature
@tgodara-aws tgodara-aws force-pushed the feature/transformation-history-table branch from 0b29f26 to 31bd370 Compare August 4, 2025 18:00
@laileni-aws
Copy link
Contributor

/retryBuilds

@laileni-aws laileni-aws closed this Aug 4, 2025
@laileni-aws laileni-aws reopened this Aug 4, 2025
@amazon-inspector-ohio
Copy link

⏳ I'm reviewing this pull request for security vulnerabilities and code quality issues. I'll provide an update when I'm done

@amazon-inspector-ohio
Copy link

✅ I finished the code review, and didn't find any security or code quality issues.

@chungjac chungjac closed this Aug 5, 2025
@chungjac chungjac reopened this Aug 5, 2025
@amazon-inspector-ohio
Copy link

⏳ I'm reviewing this pull request for security vulnerabilities and code quality issues. I'll provide an update when I'm done

@amazon-inspector-ohio
Copy link

✅ I finished the code review, and didn't find any security or code quality issues.

@chungjac
Copy link
Contributor

chungjac commented Aug 5, 2025

/retryBuilds

@chungjac chungjac merged commit 712d978 into aws:master Aug 5, 2025
30 of 31 checks passed
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.

5 participants