You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2021/04/16 15:18:04 UTC

[GitHub] [druid] machine424 opened a new pull request #11121: Make it possible to use a different S3 endpoint for ingestion

machine424 opened a new pull request #11121:
URL: https://github.com/apache/druid/pull/11121


   Hello,
   
   ### Description
   
   This adresses #10893, I've started a PR to get your attention and to show you that adding the feature won't require a lot of changes.
   
   Based on the work done in https://github.com/apache/druid/pull/9375 (I know thought that PasswordProvider has been deprecated)
   
   Let me know what you think.
   
   This PR has:
   - [ ] been self-reviewed.
      - [ ] using the [concurrency checklist](https://github.com/apache/druid/blob/master/dev/code-review/concurrency.md) (Remove this item if the PR doesn't have any relation to concurrency.)
   - [ ] added documentation for new or modified features or behaviors.
   - [ ] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
   - [ ] added or updated version, license, or notice information in [licenses.yaml](https://github.com/apache/druid/blob/master/dev/license.md)
   - [ ] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
   - [ ] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
   - [ ] added integration tests.
   - [ ] been tested in a test Druid cluster.
   


-- 
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: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] maytasm edited a comment on pull request #11121: Make it possible to use a different S3 endpoint for ingestion

Posted by GitBox <gi...@apache.org>.
maytasm edited a comment on pull request #11121:
URL: https://github.com/apache/druid/pull/11121#issuecomment-873134153


   @machine424 Thank you for looking into this. I think this approach looks reasonable. Do you imagine any other configurations that might be useful to add (in addition to the endpoint URL)? I am thinking being able to configure `region` might also be useful.


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [druid] yuanlihan commented on a change in pull request #11121: Make it possible to use a different S3 endpoint for ingestion

Posted by GitBox <gi...@apache.org>.
yuanlihan commented on a change in pull request #11121:
URL: https://github.com/apache/druid/pull/11121#discussion_r615532264



##########
File path: extensions-core/s3-extensions/src/main/java/org/apache/druid/data/input/s3/S3InputSourceConfig.java
##########
@@ -63,6 +73,7 @@ public PasswordProvider getSecretAccessKey()
     return secretAccessKey;
   }
 
+

Review comment:
       Please remove this useless blank line.




-- 
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: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] maytasm edited a comment on pull request #11121: Make it possible to use a different S3 endpoint for ingestion

Posted by GitBox <gi...@apache.org>.
maytasm edited a comment on pull request #11121:
URL: https://github.com/apache/druid/pull/11121#issuecomment-873134153






-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [druid] machine424 commented on pull request #11121: Make it possible to use a different S3 endpoint for ingestion

Posted by GitBox <gi...@apache.org>.
machine424 commented on pull request #11121:
URL: https://github.com/apache/druid/pull/11121#issuecomment-873130520


   I'll ping you @jihoonson as you've reviewed https://github.com/apache/druid/pull/9375, any thoughts about this? so I can put more effort making it ready.


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [druid] machine424 commented on pull request #11121: Make it possible to use a different S3 endpoint for ingestion

Posted by GitBox <gi...@apache.org>.
machine424 commented on pull request #11121:
URL: https://github.com/apache/druid/pull/11121#issuecomment-842191045


   I'd like to know what you think about this.


-- 
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: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] maytasm commented on pull request #11121: Make it possible to use a different S3 endpoint for ingestion

Posted by GitBox <gi...@apache.org>.
maytasm commented on pull request #11121:
URL: https://github.com/apache/druid/pull/11121#issuecomment-873134153


   @machine424 This approach looks reasonable to me. Do you image any other configurations that might be useful to add? I am thinking being able to configure region in the ingestionSpec might also be useful 


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [druid] machine424 commented on pull request #11121: Make it possible to use a different S3 endpoint for ingestion

Posted by GitBox <gi...@apache.org>.
machine424 commented on pull request #11121:
URL: https://github.com/apache/druid/pull/11121#issuecomment-842191045


   I'd like to know what you think about this.


-- 
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: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] machine424 commented on a change in pull request #11121: Make it possible to use a different S3 endpoint for ingestion

Posted by GitBox <gi...@apache.org>.
machine424 commented on a change in pull request #11121:
URL: https://github.com/apache/druid/pull/11121#discussion_r616511158



##########
File path: extensions-core/s3-extensions/src/main/java/org/apache/druid/data/input/s3/S3InputSourceConfig.java
##########
@@ -63,6 +73,7 @@ public PasswordProvider getSecretAccessKey()
     return secretAccessKey;
   }
 
+

Review comment:
       Thanks, but I said this is just a "draft", I'd like to initiate a discussion and see if this feature could be supported.




-- 
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: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] jihoonson commented on pull request #11121: Make it possible to use a different S3 endpoint for ingestion

Posted by GitBox <gi...@apache.org>.
jihoonson commented on pull request #11121:
URL: https://github.com/apache/druid/pull/11121#issuecomment-873133568


   @machine424 thanks for bringing this up. I like the goal here, will take a look at the code soon.


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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