[GitHub] [hadoop-ozone] elek commented on a change in pull request #1212: HDDS-3979. Make bufferSize configurable for stream copy

[GitBox <gi...@apache.org>]

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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org