Repository navigation
Conversation
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
gnodet-bot
left a comment
There was a problem hiding this comment.
Well-structured hardening pass across 10 AWS producers — eliminates NPE on missing operation, adds type-safe header conversion helpers with clear error messages, and fixes several secondary bugs (wrong log messages, missing result headers, emulator compatibility). Overall the code is sound. A few minor notes below.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 17 of 707 tested, 23 compile-only — current: 17 all testedMaveniverse Scalpel detected 17 affected modules (current approach: 17). Skip-tests mode would test 17 modules (14 direct + 7 downstream), skip tests for 23 (generated code, meta-modules) Modules Scalpel would test (17)
Modules with tests skipped (23)
All tested modules (44 modules, 9m 30s total)Total reactor time: 9m 30s
Top 20 slowest modules:
|
gnodet-bot
left a comment
There was a problem hiding this comment.
Test comment
This review was generated by an AI agent, Hermès on behalf of @gnodet.
gnodet-bot
left a comment
There was a problem hiding this comment.
The new commit correctly completes the null-safe AwsExceptionUtil.errorCode() migration in AWSConfigProducer and Rekognition2Producer ✅. Two points from the previous review remain open:
This review was generated by an AI agent, Hermès on behalf of @gnodet.
…int, and headers of the wrong type are no longer dropped Fixes CAMEL-26032, CAMEL-26070, CAMEL-26075, CAMEL-26076, CAMEL-26095, CAMEL-26096, CAMEL-26097, CAMEL-26098, CAMEL-26099 and CAMEL-26101. - aws2-ecs, aws2-eks, aws2-msk, aws2-mq, aws2-ec2, aws-config, aws2-step-functions, aws2-transcribe, aws2-rekognition, aws-bedrock-agent: no operation fails with an IllegalArgumentException instead of a NullPointerException - eks, msk, mq: a header of the wrong type fails instead of being dropped; mq takes a configuration id as text - msk, mq: createCluster/createBroker and describeCluster/describeBroker set the result headers with pojoRequest - ec2: the instance type by its AWS value (t2.micro), instance ids as a String separated by commas - rekognition: facial and detection attributes as Strings, the operation and header descriptions, the input image docs - step-functions: no host prefix injection with uriEndpointOverride, so startSyncExecution works against an emulator - transcribe: the phrases as a List and the vocabulary file uri, delete operations set the response, endpoints with the default or profile credentials provider - bedrock-agent: the catalog lists its own headers, not those of aws-bedrock-agent-runtime; the page describes it - aws-config: putConformancePack asks for the conformance pack name; the page describes AWS Config - msk, mq, ec2: doc examples and descriptions Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
…r no operation reads; camel-aws2-mq - management category Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
…nput must be JSON text, startSyncExecution sets the status headers; camel-aws-bedrock - aws-bedrock-agent headers override the options Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
… an AWS error without details no longer fail Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
… check InstanceType.fromValue never needs Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
9ca641e to
5ffecf5
Compare
Main's new body type text is updated where this PR changes the behavior it describes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
…int, and headers of the wrong type are no longer dropped (#28353) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
|
Merged to main as f1bb610 (squashed together with the other body type bug batches, tested together locally). Claude Code on behalf of davsclaus |
Fixes the bugs of 10 AWS components found while adding the catalog body types (CAMEL-24961):
CAMEL-26032 aws2-ec2,
CAMEL-26070 aws-bedrock-agent,
CAMEL-26075 aws2-transcribe,
CAMEL-26076 aws2-rekognition,
CAMEL-26095 aws2-mq,
CAMEL-26096 aws2-msk,
CAMEL-26097 aws2-eks,
CAMEL-26098 aws2-ecs,
CAMEL-26099 aws-config,
CAMEL-26101 aws2-step-functions.
Common to all 10
With no operation option and no operation header, the producers failed with a
NullPointerExceptionon theswitch. They now fail withIllegalArgumentException: Operation must be specified, either with the operation option or the CamelAwsXxxOperation header, as aws2-kms already did.Headers of the wrong type
A header holding a value of another type was converted to
nulland silently left out of the request. Now it is either converted, when the conversion is unambiguous, or the exchange fails with an error that names the header:CamelAwsEC2InstanceTypealso takes the AWS value (t2.micro, as the doc examples use) besides the constant name.CamelAwsEC2InstancesIdsalso takes ids separated by commas, and null ids in a list are skipped.CamelAwsMQConfigurationIDalso takes the id as a String. ACamelAwsMQBrokerUserselement that is not aUserfails, instead of aClassCastExceptionin the SDK.CamelAwsMSKBrokerNodesGroupInfo) and eks (CamelAwsEKSVPCConfig): fail.CamelAwsRekognitionFacialAttributesandCamelAwsRekognitionDetectionAttributestake the attribute names as Strings, which gave aClassCastExceptionbefore.CamelAwsStepFunctionsExecutionInputmust be JSON text (a String, a byte array or a stream). AMapwas sent as{a=1}.Per component
CamelAwsEKSDescriptionheader, which no operation read, is removed.managementcategory, as msk is.template.sendinassertDoesNotThrow, which can never fail. They now check the exchange.uriEndpointOverride, host prefix injection is disabled. The SDK otherwise sends startSyncExecution tosync-localhost(emulators).CamelAwsStepFunctionsExecutionStatusheader, andCamelAwsStepFunctionsExecutionErrorandCamelAwsStepFunctionsExecutionCausewhen the execution failed. The exchange still does not fail.CamelAwsTranscribeVocabularyFileUri; updateVocabulary sends them too.useDefaultCredentialsProvideroruseProfileCredentialsProvider.List<oftware...Attribute>.CamelAwsRekognitionClientRequestTokenhad the descriptions of other components.Imagein the headers), and the createCollection example sets the collection id.headersClass = BedrockAgentRuntimeConstants.class. So the catalog, the page and the endpoint DSL listed the 26 headers of aws-bedrock-agent-runtime and none of its own; they now list its 6.Also: CAMEL-26079 (aws-config, rekognition)
The trace logs in the catch blocks of
AWSConfigProducerandRekognition2Producerreadase.awsErrorDetails().errorCode(), which fails with aNullPointerExceptionfor an AWS error without details and hides it. They now useAwsExceptionUtil.errorCode(ase). The rest of CAMEL-26079 is in #28376; these two files are here because this PR changes them.Upgrade guide
One entry in 4.24, "camel-aws - producers without an operation, and headers of the wrong type". It covers:
Not changed
startSyncExecution does not fail the exchange when the execution failed (the status header is set instead).
Tests
New unit tests for each fix. All pass: ecs 25, eks 25, msk 31, mq 41, ec2 62, aws-config 10, step-functions 66, transcribe 53, rekognition 86, bedrock 147. The catalog JSON and docs, the endpoint DSL and the component DSL are regenerated.
Claude Code on behalf of davsclaus
🤖 Generated with Claude Code