Skip to content

feat(amazonq): enable client-side build #7226

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

Merged
merged 26 commits into from
Jun 24, 2025
Merged

feat(amazonq): enable client-side build #7226

merged 26 commits into from
Jun 24, 2025

Conversation

dhasani23
Copy link
Contributor

Problem

Instead of running mvn dependency:copy-dependencies and mvn clean install, we have a JAR that we can execute which will gather all of the project dependencies as well as some important metadata stored in a compilations.json file which our service will use to improve the quality of transformations.

Solution

Remove Maven shell commands; add custom JAR execution.


  • 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.

@dhasani23 dhasani23 requested review from a team as code owners May 5, 2025 16:42
@dhasani23 dhasani23 marked this pull request as draft May 5, 2025 16:42
const javaVersion = validProjects[0].JDKVersion ?? JDKVersion.UNSUPPORTED
telemetryJavaVersion = JDKToTelemetryValue(javaVersion) as CodeTransformJavaSourceVersionsAllowed
}
telemetry.record({ codeTransformLocalJavaVersion: telemetryJavaVersion })
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Lots of useless telemetry, so getting rid of some things and will remove them from the commons package shortly

@@ -30,12 +30,6 @@ export class NoMavenJavaProjectsFoundError extends ToolkitError {
}
}

export class ZipExceedsSizeLimitError extends ToolkitError {
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 often allowlist certain customers for ZIP size limits greater than the default of 2GB, so remove this client-side check (which didn't seem to work well anyway) and let our service fail the transformation with a relevant error message if the ZIP is too large.

await finalizeTransformByQ(status)
// bubble up error to callee function
Copy link
Contributor Author

@dhasani23 dhasani23 May 5, 2025

Choose a reason for hiding this comment

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

caller*

Our HIL (Human-in-the-Loop) feature I believe is effectively obsolete once this PR is merged; will discuss to confirm and then hopefully will be able to delete all of the dead HIL-related code here.

@@ -713,23 +703,16 @@ export async function postTransformationJob() {
const durationInMs = calculateTotalLatency(CodeTransformTelemetryState.instance.getStartTime())
const resultStatusMessage = transformByQState.getStatus()

if (transformByQState.getTransformationType() !== TransformationType.SQL_CONVERSION) {
// the below is only applicable when user is doing a Java 8/11 language upgrade
const versionInfo = await getVersionData()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was executing a shell command at the end of every transformation just to emit telemetry on the user's Maven version, which nobody even looked at.

@@ -931,10 +928,6 @@ export class TransformByQState {
return this.projectCopyFilePath
}

public getJobFailureMetadata() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was used to store request IDs when API calls fail, to show to users in the chat. In practice it's difficult to manage this since it required setJobFailureMetadata() to be executed in several places throughout the codebase, often overwriting previous request IDs, so just remove it entirely since we log the full error trace (which will include the request ID) anyway.

tempFilePath: string
fileSize: number
}

export async function zipCode(
{ dependenciesFolder, humanInTheLoopFlag, projectPath, zipManifest }: IZipCodeParams,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

unused

// We don't add build-logs.txt file to the manifest if we are
// uploading HIL artifacts
if (!humanInTheLoopFlag) {
zip.addLocalFile(logFilePath)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This log file contained the stdout of mvn dependency:copy-dependencies and mvn clean install. We included it in the upload ZIP for an earlier project which never actually happened; our backend does nothing with this file.

function installProjectDependencies(dependenciesFolder: FolderInfo, modulePath: string) {
telemetry.codeTransform_localBuildProject.run(() => {
telemetry.record({ codeTransformSessionId: CodeTransformTelemetryState.instance.getSessionId() })
function collectDependenciesAndMetadata(dependenciesFolderPath: string, workingDirPath: string) {
Copy link
Contributor Author

@dhasani23 dhasani23 May 5, 2025

Choose a reason for hiding this comment

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

This diff is a bit difficult to look at, but basically remove installProjectDependencies which ran mvn clean install, remove copyProjectDependencies which ran mvn dependency:copy-dependencies, and instead just run collectDependenciesAndMetadata which will execute the JAR we have.

throwIfCancelled()
void vscode.window.showInformationMessage(CodeWhispererConstants.buildSucceededNotification)
}

export async function getVersionData() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

See earlier comment; no point in running a shell command just to emit a metric that nobody looks at.

@dhasani23 dhasani23 changed the title feat(amazonq): enable client-side build feature, including local maven JAR execution feat(amazonq): enable client-side build feature + local maven JAR execution May 9, 2025
@dhasani23 dhasani23 changed the title feat(amazonq): enable client-side build feature + local maven JAR execution feat(amazonq): enable client-side build / run JAR May 9, 2025
@@ -214,7 +211,6 @@ export async function getJsonValuesFromManifestFile(
return {
hilCapability: jsonValues?.hilType,
pomFolderName: jsonValues?.pomFolderName,
// TODO remove this forced version
Copy link
Contributor Author

Choose a reason for hiding this comment

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

HIL-related code will be deleted shortly

@dhasani23 dhasani23 changed the title feat(amazonq): enable client-side build / run JAR feat(amazonq): enable client-side build Jun 3, 2025
@dhasani23
Copy link
Contributor Author

dhasani23 commented Jun 11, 2025

Make sure diff.patch gets parsed correctly from IDE when doing a custom upgrade via YAML (ex. snakeyaml to 2.2)

Update: working fine

@dhasani23 dhasani23 force-pushed the csbM3 branch 4 times, most recently from 32e3752 to d484af6 Compare June 19, 2025 23:16
@dhasani23 dhasani23 marked this pull request as ready for review June 20, 2025 02:34
@@ -580,7 +580,7 @@ export const invalidMetadataFileUnsupportedTargetDB =
'I can only convert SQL for migrations to Aurora PostgreSQL or Amazon RDS for PostgreSQL target databases. The provided .sct file indicates another target database for this migration.'

export const invalidCustomVersionsFileMessage =
'Your .YAML file is not formatted correctly. Make sure that the .YAML file you upload follows the format of the sample file provided.'
"I wasn't able to parse the dependency upgrade file. Check that it's configured properly and try again. For an example of the required dependency upgrade file format, see the [documentation](https://docs.aws.amazon.com/amazonq/latest/qdeveloper-ug/code-transformation.html#dependency-upgrade-file)."
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Do apostrophes work here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes because of the double quotes

@dhasani23 dhasani23 merged commit f1dc9b8 into aws:master Jun 24, 2025
31 checks passed
MarcoWang3 pushed a commit to MarcoWang3/aws-toolkit-vscode that referenced this pull request Jul 2, 2025
## Problem

Instead of running `mvn dependency:copy-dependencies` and `mvn clean
install`, we have a JAR that we can execute which will gather all of the
project dependencies as well as some important metadata stored in a
`compilations.json` file which our service will use to improve the
quality of transformations.


## Solution

Remove Maven shell commands; add custom JAR execution.


---

- 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](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines).
- License: I confirm that my contribution is made under the terms of the
Apache 2.0 license.

---------

Co-authored-by: David Hasani <davhasan@amazon.com>
l0minous pushed a commit to l0minous/aws-toolkit-vscode that referenced this pull request Jul 2, 2025
## Problem

Instead of running `mvn dependency:copy-dependencies` and `mvn clean
install`, we have a JAR that we can execute which will gather all of the
project dependencies as well as some important metadata stored in a
`compilations.json` file which our service will use to improve the
quality of transformations.


## Solution

Remove Maven shell commands; add custom JAR execution.


---

- 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](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines).
- License: I confirm that my contribution is made under the terms of the
Apache 2.0 license.

---------

Co-authored-by: David Hasani <davhasan@amazon.com>
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.

3 participants