davsclaus commented on code in PR #28256:
URL: https://github.com/apache/camel/pull/28256#discussion_r4344925556
##########
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:
Optional: `doShutdown` nulls `cloudTrailClient` (after closing it) before it
clears `lastTime`. So a shutdown during the lookups makes the next page call
fail, and the catch below logs it. A page without a start time is not what
happens. The main point of reading the field once is that both lookups and
every page of one check share one start time by construction. Maybe say just
that, for example:
```suggestion
// read lastTime once: every page of both lookups of this
check uses the same start time
Instant since = lastTime;
```
--
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]
