CAMEL-24200: camel-aws2-timestream - createScheduledQuery sets the time column correctly#24909
Merged
Conversation
…me column correctly createScheduledQuery wrote the CamelAwsTimestreamTimeColumn header into the target TimestreamConfiguration's tableName instead of its timeColumn, so the required time column was never set and, when a table-name header was also supplied, the time-column value silently overwrote the table name. Also removes a duplicated block that set scheduledQueryExecutionRoleArn twice from the same header. Extends Timestream2QueryProducerTest to capture the CreateScheduledQuery request and assert the database name, table name and time column are all set from their own headers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Andrea Cosentino <ancosen@gmail.com>
Contributor
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
gnodet
approved these changes
Jul 19, 2026
gnodet
left a comment
gnodet
left a comment
Contributor
There was a problem hiding this comment.
LGTM — clear, well-tested bug fix.
The bug: timestreamConfigBuilder.tableName(timeColumn) should be .timeColumn(timeColumn). This meant the time column was never set, and when both CamelAwsTimestreamTableName and CamelAwsTimestreamTimeColumn headers were supplied, the time-column value silently overwrote the table name. Straightforward copy-paste error.
The fix:
tableName(timeColumn)→timeColumn(timeColumn)— correct builder method ✅- Removed duplicate
scheduledQueryExecutionRoleArnblock (confirmed: lines 178 and 205 onmainare identical; the PR removes the second) ✅
The test: Capturing CreateScheduledQueryRequest in the mock client and asserting that databaseName, tableName, and timeColumn each map to their respective headers is the right pattern. The test would fail without the fix (tableName → "time", timeColumn → null).
No concerns.
Claude Code on behalf of gnodet — AI-generated review
Contributor
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 9 tested, 29 compile-only — current: 9 all testedMaveniverse Scalpel detected 38 affected modules (current approach: 9).
|
davsclaus
approved these changes
Jul 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes CAMEL-24200. In
Timestream2QueryProducer.createScheduledQuerythe time-column header was written into the target configuration's table name:So the
timeColumn(required on a scheduled-query target) was never set, and whenCamelAwsTimestreamTableNamewas also supplied the time-column value silently overwrote the table name — creating the scheduled query against the wrong table, or having AWS reject it.Also removes a duplicated block that set
scheduledQueryExecutionRoleArntwice from the same header.Tests
Extends the existing
Timestream2QueryProducerTest.timestreamCreateScheduledQueryTest(which already sent all three headers but only asserted the response ARN) to capture theCreateScheduledQueryRequestvia the client mock and assertdatabaseName,tableNameandtimeColumneach come from their own header. Against the old code the table name would be"time"and the time columnnull, so the test fails without the fix.Backport
Same code on
camel-4.18.xandcamel-4.14.x; will be backported after merge (fixVersions 4.22.0 / 4.18.4 / 4.14.9).Claude Code on behalf of oscerd