You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/08/12 00:36:35 UTC
[GitHub] [iceberg] xiaoxuandev opened a new pull request, #5508: AWS: Add S3 preload client configuration
xiaoxuandev opened a new pull request, #5508:
URL: https://github.com/apache/iceberg/pull/5508
Adding support for Initializing S3 client during the S3FileIO initialization. This is needed to deal with a scenario in which S3 files need to be deleted after table has been dropped when LakeFormation is enabled.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] ericlgoodman commented on pull request #5508: AWS: Support preload S3 client mode for S3FileIO
Posted by GitBox <gi...@apache.org>.
ericlgoodman commented on PR #5508:
URL: https://github.com/apache/iceberg/pull/5508#issuecomment-1215638776
Looks good to me!
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] jackye1995 commented on pull request #5508: AWS: Support preload S3 client mode for S3FileIO
Posted by GitBox <gi...@apache.org>.
jackye1995 commented on PR #5508:
URL: https://github.com/apache/iceberg/pull/5508#issuecomment-1212807565
Oh actually nvm, @singhpk234 I get what you mean now, yes the part in S3FileIO should be included. @xiaoxuandev let us know when you have that part added.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] jackye1995 commented on a diff in pull request #5508: AWS: Add S3 preload client configuration
Posted by GitBox <gi...@apache.org>.
jackye1995 commented on code in PR #5508:
URL: https://github.com/apache/iceberg/pull/5508#discussion_r944036770
##########
aws/src/main/java/org/apache/iceberg/aws/AwsProperties.java:
##########
@@ -389,6 +389,19 @@ public class AwsProperties implements Serializable {
*/
@Deprecated public static final boolean CLIENT_ENABLE_ETAG_CHECK_DEFAULT = false;
+ /**
+ * This flag controls whether the S3 client will be initialized during the S3FileIO
+ * initialization, instead of normal lazy initialization upon use. This is needed for cases that
+ * the credentials to use might change and needs to be preloaded.
+ *
+ * <p>In {@link GlueCatalog#newTableOps}, we will set this flag to true if 1) this table exists
Review Comment:
this paragraph is not needed.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] singhpk234 commented on pull request #5508: AWS: Support preload S3 client mode for S3FileIO
Posted by GitBox <gi...@apache.org>.
singhpk234 commented on PR #5508:
URL: https://github.com/apache/iceberg/pull/5508#issuecomment-1212774963
should we include the code part which actually does [s3 client initialization](https://github.com/apache/iceberg/pull/4695/files#diff-322d2e2be6cbad87106f05c0456cf953d99f396c71c99a1d8c7bc281022f8b6bR342) based on this property in this PR ?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] singhpk234 commented on a diff in pull request #5508: AWS: Support preload S3 client mode for S3FileIO
Posted by GitBox <gi...@apache.org>.
singhpk234 commented on code in PR #5508:
URL: https://github.com/apache/iceberg/pull/5508#discussion_r944158222
##########
aws/src/main/java/org/apache/iceberg/aws/AwsProperties.java:
##########
@@ -389,6 +389,15 @@ public class AwsProperties implements Serializable {
*/
@Deprecated public static final boolean CLIENT_ENABLE_ETAG_CHECK_DEFAULT = false;
+ /**
+ * This flag controls whether the S3 client will be initialized during the S3FileIO
+ * initialization, instead of normal lazy initialization upon use. This is needed for cases that
Review Comment:
[nit]
```suggestion
* initialization, instead of default lazy initialization upon use. This is needed for cases that
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] jackye1995 merged pull request #5508: AWS: Support preload S3 client mode for S3FileIO
Posted by GitBox <gi...@apache.org>.
jackye1995 merged PR #5508:
URL: https://github.com/apache/iceberg/pull/5508
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] jackye1995 commented on a diff in pull request #5508: AWS: Add S3 preload client configuration
Posted by GitBox <gi...@apache.org>.
jackye1995 commented on code in PR #5508:
URL: https://github.com/apache/iceberg/pull/5508#discussion_r944040003
##########
aws/src/main/java/org/apache/iceberg/aws/AwsProperties.java:
##########
@@ -422,6 +435,8 @@ public class AwsProperties implements Serializable {
private boolean glueCatalogSkipNameValidation;
private boolean glueLakeFormationEnabled;
+ private boolean s3PreloadClientEnabled;
Review Comment:
nit: put together with other s3 config variables
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] xiaoxuandev commented on a diff in pull request #5508: AWS: Support preload S3 client mode for S3FileIO
Posted by GitBox <gi...@apache.org>.
xiaoxuandev commented on code in PR #5508:
URL: https://github.com/apache/iceberg/pull/5508#discussion_r944633283
##########
aws/src/main/java/org/apache/iceberg/aws/AwsProperties.java:
##########
@@ -389,6 +389,15 @@ public class AwsProperties implements Serializable {
*/
@Deprecated public static final boolean CLIENT_ENABLE_ETAG_CHECK_DEFAULT = false;
+ /**
+ * This flag controls whether the S3 client will be initialized during the S3FileIO
+ * initialization, instead of normal lazy initialization upon use. This is needed for cases that
Review Comment:
Good point, fixed, thanks!
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] jackye1995 commented on pull request #5508: AWS: Support preload S3 client mode for S3FileIO
Posted by GitBox <gi...@apache.org>.
jackye1995 commented on PR #5508:
URL: https://github.com/apache/iceberg/pull/5508#issuecomment-1212806558
> should we also include the code part which actually does [s3 client initialization](https://github.com/apache/iceberg/pull/4695/files#diff-322d2e2be6cbad87106f05c0456cf953d99f396c71c99a1d8c7bc281022f8b6bR342) based on this property in this PR ?
good point, I discussed and we want to separate that part because it requires merging the LakeFormation caching PR #4695 first. This PR is to reduce the volume of that one.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] jackye1995 commented on pull request #5508: AWS: Support preload S3 client mode for S3FileIO
Posted by GitBox <gi...@apache.org>.
jackye1995 commented on PR #5508:
URL: https://github.com/apache/iceberg/pull/5508#issuecomment-1215748194
Thanks for all the reviews, looks like we have enough votes. @xiaoxuandev thanks for the contribution.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org