Skip to content

Conversation

@dhasani23
Copy link
Contributor

@dhasani23 dhasani23 commented Oct 21, 2024

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Description

QCT wants to support converting embedded SQL in Java apps.

Checklist

  • My code follows the code style of this project
  • I have added tests to cover my changes
  • A short description of the change has been added to the CHANGELOG if the change is customer-facing in the IDE.
  • I have added metrics for my changes (if required)

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 October 21, 2024 22:56
@dhasani23 dhasani23 changed the title feat(amazonq): enable SQL conversions feat(amazonq): support SQL conversions Oct 21, 2024
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

QDJVMC found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

@dhasani23 dhasani23 force-pushed the dmsWork branch 2 times, most recently from deb935c to d464830 Compare October 24, 2024 01:54
) {
maybeAdd(ProgressStepId.UPLOADING, message("codemodernizer.toolwindow.progress.uploading"))
maybeAdd(ProgressStepId.BUILDING, message("codemodernizer.toolwindow.progress.building"))
maybeAddTransformationStep(ProgressStepId.UPLOADING, message("codemodernizer.toolwindow.progress.uploading"))

Check warning

Code scanning / QDJVMC

Usage of redundant or deprecated syntax or deprecated symbols Warning

'message(String, vararg Any): String' is deprecated. Use extension-specific localization bundle instead
maybeAdd(ProgressStepId.BUILDING, message("codemodernizer.toolwindow.progress.building"))
maybeAdd(ProgressStepId.PLANNING, message("codemodernizer.toolwindow.progress.planning"))
maybeAdd(ProgressStepId.TRANSFORMING, message("codemodernizer.toolwindow.progress.transforming"))
maybeAddTransformationStep(ProgressStepId.UPLOADING, message("codemodernizer.toolwindow.progress.uploading"))

Check warning

Code scanning / QDJVMC

Usage of redundant or deprecated syntax or deprecated symbols Warning

'message(String, vararg Any): String' is deprecated. Use extension-specific localization bundle instead
maybeAddTransformationStep(ProgressStepId.UPLOADING, message("codemodernizer.toolwindow.progress.uploading"))
maybeAddTransformationStep(ProgressStepId.BUILDING, message("codemodernizer.toolwindow.progress.building"))
maybeAddTransformationStep(ProgressStepId.PLANNING, message("codemodernizer.toolwindow.progress.planning"))
maybeAddTransformationStep(ProgressStepId.TRANSFORMING, message("codemodernizer.toolwindow.progress.transforming"))

Check warning

Code scanning / QDJVMC

Usage of redundant or deprecated syntax or deprecated symbols Warning

'message(String, vararg Any): String' is deprecated. Use extension-specific localization bundle instead
loadingPanelText = if (transformType == CodeTransformType.SQL_CONVERSION) {
message("codemodernizer.toolwindow.scan_in_progress.transforming")
} else {
message("codemodernizer.toolwindow.scan_in_progress.planning")

Check warning

Code scanning / QDJVMC

Usage of redundant or deprecated syntax or deprecated symbols Warning

'message(String, vararg Any): String' is deprecated. Use extension-specific localization bundle instead
TransformationStatus.PLANNED -> {
loadingPanelText = message("codemodernizer.toolwindow.scan_in_progress.planning")
loadingPanelText = if (transformType == CodeTransformType.SQL_CONVERSION) {
message("codemodernizer.toolwindow.scan_in_progress.transforming")

Check warning

Code scanning / QDJVMC

Usage of redundant or deprecated syntax or deprecated symbols Warning

'message(String, vararg Any): String' is deprecated. Use extension-specific localization bundle instead

fun validateSctMetadata(sctFile: File?): SqlMetadataValidationResult {
if (sctFile == null) {
return SqlMetadataValidationResult(false, message("codemodernizer.chat.message.validation.error.missing_sct_file"))

Check warning

Code scanning / QDJVMC

Usage of redundant or deprecated syntax or deprecated symbols Warning

'message(String, vararg Any): String' is deprecated. Use extension-specific localization bundle instead
val targetDbServer = targets["DbServer"] as Map<*, *>
val targetVendor = (targetDbServer["vendor"] as String).trim().uppercase()
if (targetVendor != "AURORA_POSTGRESQL" && targetVendor != "RDS_POSTGRESQL") {
return SqlMetadataValidationResult(false, message("codemodernizer.chat.message.validation.error.invalid_target_db"))

Check warning

Code scanning / QDJVMC

Usage of redundant or deprecated syntax or deprecated symbols Warning

'message(String, vararg Any): String' is deprecated. Use extension-specific localization bundle instead
return SqlMetadataValidationResult(true, "", sourceVendor, targetVendor, sourceServerName, schemaNames)
} catch (e: Exception) {
getLogger<CodeTransformChatController>().error { "Error parsing .sct metadata file: $e" }
return SqlMetadataValidationResult(false, message("codemodernizer.chat.message.validation.error.invalid_sct"))

Check warning

Code scanning / QDJVMC

Usage of redundant or deprecated syntax or deprecated symbols Warning

'message(String, vararg Any): String' is deprecated. Use extension-specific localization bundle instead
@dhasani23 dhasani23 force-pushed the dmsWork branch 2 times, most recently from ad1db16 to 1e9ddf8 Compare October 31, 2024 22:30
@dhasani23 dhasani23 requested a review from a team as a code owner November 9, 2024 00:22
@dhasani23 dhasani23 marked this pull request as draft November 9, 2024 00:25
@dhasani23
Copy link
Contributor Author

TODO: add changelog entry

private val confirmUserSelectionSQLConversionMetadataButton = Button(
keepCardAfterClick = true,
waitMandatoryFormItems = true,
text = message("codemodernizer.chat.message.button.select_sql_metadata"),

Check warning

Code scanning / QDJVMC

Usage of redundant or deprecated syntax or deprecated symbols Warning

'message(String, vararg Any): String' is deprecated. Use extension-specific localization bundle instead
""".trimIndent()

private fun getUserSQLConversionSelectionFormattedMarkdown(moduleName: String, schema: String) = """
### ${message("codemodernizer.chat.prompt.title.details")}

Check warning

Code scanning / QDJVMC

Usage of redundant or deprecated syntax or deprecated symbols Warning

'message(String, vararg Any): String' is deprecated. Use extension-specific localization bundle instead
sctMetadata = xmlMapper.readValue<SctMetadata>(sctFile)
} catch (e: Exception) {
getLogger<CodeTransformChatController>().error { "Error parsing .sct metadata file; invalid XML encountered. $e" }
return SqlMetadataValidationResult(false, message("codemodernizer.chat.message.validation.error.invalid_sct"))

Check warning

Code scanning / QDJVMC

Usage of redundant or deprecated syntax or deprecated symbols Warning

'message(String, vararg Any): String' is deprecated. Use extension-specific localization bundle instead
@dhasani23 dhasani23 requested a review from a team as a code owner November 14, 2024 19:52
private const val METRICS_FILE_NAME = "metrics.json"
val LOG = getLogger<CodeModernizerArtifact>()
val MAPPER = jacksonObjectMapper()
val XML_MAPPER = XmlMapper().registerKotlinModule().configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false)

Check notice

Code scanning / QDJVMC

Function or property has platform type Note

Declaration has type inferred from a platform call, which can lead to unchecked nullability issues. Specify type explicitly as nullable or non-nullable.
fun buildSQLMetadataValidationSuccessDetailsChatContent(validationResult: SqlMetadataValidationResult) = CodeTransformChatMessageContent(
type = CodeTransformChatMessageType.FinalizedAnswer,
message = """
### ${message("codemodernizer.chat.prompt.title.details")}

Check warning

Code scanning / QDJVMC

Usage of redundant or deprecated syntax or deprecated symbols Warning

'message(String, vararg Any): String' is deprecated. Use extension-specific localization bundle instead
}

override suspend fun processCodeTransformSelectSQLMetadataAction(message: IncomingCodeTransformMessage.CodeTransformSelectSQLMetadata) {
runInEdt {
Copy link
Contributor

Choose a reason for hiding this comment

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

this is suspend, so you can just do withContext(EDT) {

val metadataValidationResult = validateSctMetadata(sctFile)

if (!metadataValidationResult.valid) {
CoroutineScope(EDT).launch {
Copy link
Contributor

Choose a reason for hiding this comment

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

we are already on EDT, so what is this doing?

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 withContext(EDT) means that I no longer need to do this, previously was getting an error, fixed

val metadataValidationResult = validateSctMetadata(sctFile)

if (!metadataValidationResult.valid) {
CoroutineScope(EDT).launch {
Copy link
Contributor

@rli rli Nov 18, 2024

Choose a reason for hiding this comment

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

also this breaks structured concurrency, should be withContext(...)/launch(...) depending on desired behavior

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed entirely in favor of withContext

try {
sctMetadata = XML_MAPPER.readValue<SctMetadata>(sctFile)
} catch (e: Exception) {
getLogger<CodeTransformChatController>().error { "Error parsing .sct metadata file; invalid XML encountered. $e" }
Copy link
Contributor

Choose a reason for hiding this comment

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

our linter is broken and not catching this for some reason

Suggested change
getLogger<CodeTransformChatController>().error { "Error parsing .sct metadata file; invalid XML encountered. $e" }
getLogger<CodeTransformChatController>(e).error { "Error parsing .sct metadata file; invalid XML encountered." }

Copy link
Contributor Author

@dhasani23 dhasani23 Nov 18, 2024

Choose a reason for hiding this comment

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

When I tried this, got errors prompting me to make changes in LogUtils.kt and some other shared/common files, wasn't sure if I should do that, ideally that can be a follow up, for now I changed it to just log the localizedMessage rather than dumping the entire Error object

}
return SqlMetadataValidationResult(true, "", sourceVendor, targetVendor, sourceServerName, schemaNames)
} catch (e: Exception) {
getLogger<CodeTransformChatController>().error { "Error parsing .sct metadata file: $e" }
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
getLogger<CodeTransformChatController>().error { "Error parsing .sct metadata file: $e" }
getLogger<CodeTransformChatController>(e).error { "Error parsing .sct metadata file" }

@rli rli merged commit 0e94d02 into aws:main Nov 18, 2024
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet