jihoonson commented on a change in pull request #8857: Add reference to 
druid.storage.type
URL: https://github.com/apache/incubator-druid/pull/8857#discussion_r345454907
 
 

 ##########
 File path: docs/development/extensions-core/s3.md
 ##########
 @@ -61,6 +63,7 @@ As an example, to set the region to 'us-east-1' through 
system properties:
 |`druid.storage.sse.type`|Server-side encryption type. Should be one of `s3`, 
`kms`, and `custom`. See the below [Server-side encryption 
section](#server-side-encryption) for more details.|None|
 |`druid.storage.sse.kms.keyId`|AWS KMS key ID. This is used only when 
`druid.storage.sse.type` is `kms` and can be empty to use the default key 
ID.|None|
 |`druid.storage.sse.custom.base64EncodedKey`|Base64-encoded key. Should be 
specified if `druid.storage.sse.type` is `custom`.|None|
+|`druid.storage.type`|Global deep storage provider. Must be set to `s3` to 
make use of this extension.|`local`| 
 
 Review comment:
   The default value is `local` and so this doc is technically correct. But, 
I'm a bit worried about whether it could confuse people. How about changing it 
to `Must be set.` instead of `local`?
   
   Also, I guess you added it here based on an alphabetical order but there's 
no actual ordering I think. How about moving it to the top of the table since 
it must be set?

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org

Reply via email to