Skip to content

Commit 271b04f

Browse files
committed
Cosmetic fixes to repository-s3 (elastic#125397)
Relates AWS SDK v2 uprgade, this commit just pulls out some bits that can go in first.
1 parent 552d485 commit 271b04f

File tree

2 files changed

+18
-10
lines changed

2 files changed

+18
-10
lines changed

modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3ClientSettings.java

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -122,28 +122,28 @@ final class S3ClientSettings {
122122
static final Setting.AffixSetting<TimeValue> READ_TIMEOUT_SETTING = Setting.affixKeySetting(
123123
PREFIX,
124124
"read_timeout",
125-
key -> Setting.timeSetting(key, TimeValue.timeValueMillis(ClientConfiguration.DEFAULT_SOCKET_TIMEOUT), Property.NodeScope)
125+
key -> Setting.timeSetting(key, Defaults.READ_TIMEOUT, Property.NodeScope)
126126
);
127127

128128
/** The maximum number of concurrent connections to use. */
129129
static final Setting.AffixSetting<Integer> MAX_CONNECTIONS_SETTING = Setting.affixKeySetting(
130130
PREFIX,
131131
"max_connections",
132-
key -> Setting.intSetting(key, ClientConfiguration.DEFAULT_MAX_CONNECTIONS, 1, Property.NodeScope)
132+
key -> Setting.intSetting(key, Defaults.MAX_CONNECTIONS, 1, Property.NodeScope)
133133
);
134134

135135
/** The number of retries to use when an s3 request fails. */
136136
static final Setting.AffixSetting<Integer> MAX_RETRIES_SETTING = Setting.affixKeySetting(
137137
PREFIX,
138138
"max_retries",
139-
key -> Setting.intSetting(key, ClientConfiguration.DEFAULT_RETRY_POLICY.getMaxErrorRetry(), 0, Property.NodeScope)
139+
key -> Setting.intSetting(key, Defaults.RETRY_COUNT, 0, Property.NodeScope)
140140
);
141141

142142
/** Whether retries should be throttled (ie use backoff). */
143143
static final Setting.AffixSetting<Boolean> USE_THROTTLE_RETRIES_SETTING = Setting.affixKeySetting(
144144
PREFIX,
145145
"use_throttle_retries",
146-
key -> Setting.boolSetting(key, ClientConfiguration.DEFAULT_THROTTLE_RETRIES, Property.NodeScope)
146+
key -> Setting.boolSetting(key, Defaults.THROTTLE_RETRIES, Property.NodeScope)
147147
);
148148

149149
/** Whether the s3 client should use path style access. */
@@ -336,7 +336,7 @@ S3ClientSettings refine(Settings repositorySettings) {
336336

337337
/**
338338
* Load all client settings from the given settings.
339-
*
339+
* <p>
340340
* Note this will always at least return a client named "default".
341341
*/
342342
static Map<String, S3ClientSettings> load(Settings settings) {
@@ -502,4 +502,11 @@ private static <T> T getRepoSettingOrDefault(Setting.AffixSetting<T> setting, Se
502502
}
503503
return defaultValue;
504504
}
505+
506+
static final class Defaults {
507+
static final TimeValue READ_TIMEOUT = TimeValue.timeValueMillis(ClientConfiguration.DEFAULT_SOCKET_TIMEOUT);
508+
static final int MAX_CONNECTIONS = ClientConfiguration.DEFAULT_MAX_CONNECTIONS;
509+
static final int RETRY_COUNT = ClientConfiguration.DEFAULT_RETRY_POLICY.getMaxErrorRetry();
510+
static final boolean THROTTLE_RETRIES = ClientConfiguration.DEFAULT_THROTTLE_RETRIES;
511+
}
505512
}

modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Service.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,8 @@ class S3Service implements Closeable {
111111

112112
/**
113113
* Refreshes the settings for the AmazonS3 clients and clears the cache of
114-
* existing clients. New clients will be build using these new settings. Old
115-
* clients are usable until released. On release they will be destroyed instead
114+
* existing clients. New clients will be built using these new settings. Old
115+
* clients are usable until released. On release, they will be destroyed instead
116116
* of being returned to the cache.
117117
*/
118118
public synchronized void refreshAndClearCache(Map<String, S3ClientSettings> clientsSettings) {
@@ -122,7 +122,7 @@ public synchronized void refreshAndClearCache(Map<String, S3ClientSettings> clie
122122
this.staticClientSettings = Maps.ofEntries(clientsSettings.entrySet());
123123
derivedClientSettings = emptyMap();
124124
assert this.staticClientSettings.containsKey("default") : "always at least have 'default'";
125-
// clients are built lazily by {@link client}
125+
/* clients are built lazily by {@link #client} */
126126
}
127127

128128
/**
@@ -330,7 +330,8 @@ public void refresh() {
330330
* <ul>
331331
* <li>Reads the the location of the web identity token not from AWS_WEB_IDENTITY_TOKEN_FILE, but from a symlink
332332
* in the plugin directory, so we don't need to create a hardcoded read file permission for the plugin.</li>
333-
* <li>Supports customization of the STS endpoint via a system property, so we can test it against a test fixture.</li>
333+
* <li>Supports customization of the STS (Security Token Service) endpoint via a system property, so we can
334+
* test it against a test fixture.</li>
334335
* <li>Supports gracefully shutting down the provider and the STS client.</li>
335336
* </ul>
336337
*/
@@ -373,7 +374,7 @@ static class CustomWebIdentityTokenCredentialsProvider implements AWSCredentials
373374
if (roleArn == null) {
374375
LOGGER.warn(
375376
"Unable to use a web identity token for authentication. The AWS_WEB_IDENTITY_TOKEN_FILE environment "
376-
+ "variable is set, but either AWS_ROLE_ARN is missing"
377+
+ "variable is set, but AWS_ROLE_ARN is missing"
377378
);
378379
return;
379380
}

0 commit comments

Comments
 (0)