allthingssecurity commented on code in PR #28256:
URL: https://github.com/apache/camel/pull/28256#discussion_r4345341532
##########
components/camel-aws/camel-aws-secrets-manager/src/main/java/org/apache/camel/component/aws/secretsmanager/vault/CloudTrailReloadTriggerTask.java:
##########
@@ -314,13 +317,16 @@ public void run() {
// read every page of the update events since the last check.
LookupEvents takes a single lookup
// attribute, so there is one lookup per event name: filtering
on the event source instead would also
// return every read (GetSecretValue, DescribeSecret), and the
pages would grow with the read traffic
+ // read lastTime once: every page of both lookups uses the
same start time, even if the task is shut
+ // down (which clears lastTime) while the lookups run
+ Instant since = lastTime;
Review Comment:
Agreed, thanks: `doShutdown` closes and clears the client first, so a page
sent without a start time is not what happens on a shutdown. I took your
wording for the comment. The test comment now says only that `lastTime` changes
while the first page is read. The PR description no longer gives the shutdown
case as the reason.
_Claude Code on behalf of allthingssecurity_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
