Add support for reloading Gcs credentials #759
Conversation
Gcs credentials are automatically reloaded if gcs.credentials.path is used to define the credentials in a separate json file.
tvainika
force-pushed
the
tvainika/reloading-gcs-credentials
branch
from
ca7fc71 to
59d0746
Compare
| * Closes the credentials provider to stop file watching. | ||
| * This should be called when the storage backend is being shut down. | ||
| */ | ||
| public void close() { |
There was a problem hiding this comment.
This close is not being called from anywhere else I think. We should call it to avoid leaving resources hanging.
There was a problem hiding this comment.
I see S3 version also doesn't have a close method. May be worth adding it there also.
There was a problem hiding this comment.
In my understanding S3 works so that S3Client (implemented in AWS SDK) receives an instance of S3RotatingCredentialsProvider, which implements AutoCloseable, and then it is S3Client's responsibility to call that close. So in there SDK does a bit more if I've understood correctly.
But you seem to be right that here this close is not being called. I think this might need some changes in outside Gcs parts, right?
There was a problem hiding this comment.
Yes, we need something similar to aiven/inkless#430
| this.scheduledExecutorService = Executors.newSingleThreadScheduledExecutor(r -> { | ||
| final Thread t = new Thread(r, "s3-credentials-watcher"); | ||
| t.setDaemon(true); | ||
| return t; | ||
| }); |
There was a problem hiding this comment.
Could you elaborate on why is this needed?
Also, maybe a separate commit?
There was a problem hiding this comment.
Added just naming of the thread for consistency. But true, should be separate commit.
2 participants
Gcs credentials are automatically reloaded if gcs.credentials.path is used to define the credentials in a separate json file.