elek commented on a change in pull request #1212:
URL: https://github.com/apache/hadoop-ozone/pull/1212#discussion_r462307639
##########
File path:
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java
##########
@@ -114,6 +120,13 @@ public ObjectEndpoint() {
customizableGetHeaders.add("Content-Encoding");
}
+ @PostConstruct
+ public void init() {
Review comment:
NIT: It can be slightly better to use
```
@Inject
private OzoneConfiguration ozoneConfiguration;
```
And you don't need to expose `getClient()` in the `EndpointBase`.
(Didn't try, but it should work, IMHO).
##########
File path:
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java
##########
@@ -465,6 +465,12 @@
public static final String OZONE_CLIENT_HTTPS_NEED_AUTH_KEY =
"ozone.https.client.need-auth";
public static final boolean OZONE_CLIENT_HTTPS_NEED_AUTH_DEFAULT = false;
+
Review comment:
Two things:
* `ozone.client.buffer.size` seems to be confusing. This configuration for
the Ozone client of the **s3g**. As we have a global list of configuration
keys, I guess we need a `s3g` prefix here. (For example
`ozone.s3g.client.buffer.size`)
* Usually, I prefer to use annotation based configuration, if possible. (It
can be harder here as S3g uses CDI injections. If it's too complex, you can
ignore...)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
