-
Notifications
You must be signed in to change notification settings - Fork 603
feat(amazonq): enable client-side build / run JAR #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
base: master
Are you sure you want to change the base?
Conversation
const javaVersion = validProjects[0].JDKVersion ?? JDKVersion.UNSUPPORTED | ||
telemetryJavaVersion = JDKToTelemetryValue(javaVersion) as CodeTransformJavaSourceVersionsAllowed | ||
} | ||
telemetry.record({ codeTransformLocalJavaVersion: telemetryJavaVersion }) |
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.
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 { |
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.
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 |
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.
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() |
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.
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() { |
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.
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, |
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.
unused
// We don't add build-logs.txt file to the manifest if we are | ||
// uploading HIL artifacts | ||
if (!humanInTheLoopFlag) { | ||
zip.addLocalFile(logFilePath) |
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.
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) { |
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.
This diff is a bit difficult to look at it, 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() { |
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.
See earlier comment; no point in running a shell command just to emit a metric that nobody looks at.
packages/core/src/amazonqGumby/chat/controller/messenger/messenger.ts
Outdated
Show resolved
Hide resolved
packages/core/src/test/codewhisperer/commands/transformByQ.test.ts
Outdated
Show resolved
Hide resolved
@@ -214,7 +211,6 @@ export async function getJsonValuesFromManifestFile( | |||
return { | |||
hilCapability: jsonValues?.hilType, | |||
pomFolderName: jsonValues?.pomFolderName, | |||
// TODO remove this forced version |
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.
HIL-related code will be deleted shortly
Problem
Instead of running
mvn dependency:copy-dependencies
andmvn 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 acompilations.json
file which our service will use to improve the quality of transformations.Solution
Remove Maven shell commands; add custom JAR execution.
feature/x
branches will not be squash-merged at release time.