-
Notifications
You must be signed in to change notification settings - Fork 275
fix(amazonq): show build logs when pre-build fails #5173
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
Conversation
| jobId, | ||
| sessionContext.targetJavaVersion | ||
| ) | ||
| result.state == TransformationStatus.PARTIALLY_COMPLETED -> CodeModernizerJobCompletedResult.JobPartiallySucceeded(jobId) |
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 were not using sessionContext.targetJavaVersion
| ) | ||
|
|
||
| fun buildTransformResultChatContent(result: CodeModernizerJobCompletedResult, totalPatchFiles: Int): CodeTransformChatMessageContent { | ||
| fun buildTransformResultChatContent(result: CodeModernizerJobCompletedResult, totalPatchFiles: Int? = null): CodeTransformChatMessageContent { |
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.
If the transformation fails, no patch files, so this can be optional as it's only used for full or partial successes.
| when (result) { | ||
| is CodeModernizerJobCompletedResult.Stopped, CodeModernizerJobCompletedResult.JobAbortedBeforeStarting -> handleCodeTransformStoppedByUser() | ||
| is CodeModernizerJobCompletedResult.JobFailed -> handleCodeTransformJobFailed(result.failureReason) | ||
| is CodeModernizerJobCompletedResult.JobFailedInitialBuild -> handleCodeTransformJobFailedPreBuild(result) |
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.
These make sure that 1) when the job fails, the last chat item is updated to say so and 2) when the server-side build fails, a new chat item is sent to say so + include a button to download the build logs (this button functionality was previously implemented in buildTransformResultChatContent, but it was not running).
| timeTaken | ||
| ) | ||
| } | ||
| } catch (exception: AlreadyDisposedException) { |
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 already catch this exception in the caller.
| fun setJobRunningUI() = setUI { | ||
| add(BorderLayout.CENTER, buildProgressSplitterPanelManager) | ||
| banner.updateContent(message("codemodernizer.toolwindow.banner.job_is_running"), AllIcons.General.BalloonInformation) | ||
| updateJobId() |
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.
Make job ID more visible in Transformation Hub by putting it at the top next to the job run time.
| codemodernizer.chat.message.button.view_summary=View summary | ||
| codemodernizer.chat.message.changes_applied=I applied the changes to your project. | ||
| codemodernizer.chat.message.choose_objective=I can help you with the following tasks:\n- Upgrade your Java 8 and Java 11 codebases to Java 17, or upgrade Java 17 code with up to date libraries and other dependencies.\n- Convert embedded SQL code for Oracle to PostgreSQL database migrations in AWS DMS.\n\nWhat would you like to do? You can enter "language upgrade" or "sql conversion". | ||
| codemodernizer.chat.message.choose_objective_placeholder=Enter "language upgrade" or "sql conversion" |
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 String and those below were already approved.
| codemodernizer.notification.info.download.started.title=Download Started | ||
| codemodernizer.notification.info.modernize_complete.content=Amazon Q finished the transformation. You can review the diff to see the proposed changes and accept or reject them. The transformation summary has details about the files that were updated. | ||
| codemodernizer.notification.info.modernize_complete.title=Transform Complete | ||
| codemodernizer.notification.info.modernize_complete.view_diff=View diff |
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.
Assuming these and the rest deleted were not used?
| border = BorderFactory.createEmptyBorder(0, 5, 0, 0) | ||
| } | ||
|
|
||
| private val infoLabelJobId = JBLabel().apply { |
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.
Sorry just for my own understanding, this is for the ux of displaying the job id in transformation hub right
| } catch (exception: AlreadyDisposedException) { | ||
| LOG.warn { "Disposed when about to create the loading panel" } | ||
| return | ||
| infoLabelRunningTime.text = if (runTime != null) { |
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.
Again here just curious, what running time is it referring to?
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.
The "42s" in the image above
* fix(amazonq): show build logs when pre-build fails * fix detekt errors * remove unused imports * fix detektTest --------- Co-authored-by: David Hasani <[email protected]>
* fix(amazonq): show build logs when pre-build fails * fix detekt errors * remove unused imports * fix detektTest --------- Co-authored-by: David Hasani <[email protected]>

Types of changes
Description
When the server-side build fails, the pre-build logs were being downloaded but not actually shown to user.
When the transformation fails, we were unnecessarily attempting to download the results.
Make job ID more visible in Transformation Hub.
Checklist
License
I confirm that my contribution is made under the terms of the Apache 2.0 license.