-
Notifications
You must be signed in to change notification settings - Fork 747
fix(amazonq): do not overwrite failure reason, use lowercase lang name #6055
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
|
|
Note: I may add a feature flag around this SQL Conversions feature in this PR too, will let you know by the early afternoon of Wednesday 11/20, if so I should also remove the CHANGELOG entry. |
jpinkney-aws
left a comment
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.
Also, the failure reason returned by one of our APIs was being overwritten and not shown to users.
Is that something that would be useful to have a changelog item for?
| transformByQState.getTransformationType() === TransformationType.LANGUAGE_UPGRADE | ||
| ? 'JAVA' | ||
| : 'SQL', | ||
| ? 'java' |
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 there is potential that these values will be used in other telemetry events this may be worth putting in to a 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.
Yeah, post-re:Invent planning on adding a separate utils file for these kinds of things
Problem
Our backend is expecting the language name (
javaorsql) to be lowercase, not uppercase. Also, the failure reason returned by one of our APIs was being overwritten and not shown to users.Solution
User lowercase, and prevent overwriting.
License: I confirm that my contribution is made under the terms of the Apache 2.0 license.