[GitHub] [incubator-druid] jihoonson commented on a change in pull request #8857: Add reference to druid.storage.type

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

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


With regards,
Apache Git Services

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