You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/06/28 17:21:44 UTC

[GitHub] [arrow] pitrou opened a new pull request, #13455: ARROW-16864: [Python] Allow omitting S3 external_id and session_name with role_arn

pitrou opened a new pull request, #13455:
URL: https://github.com/apache/arrow/pull/13455

   Both external_id and session_name are optional parameters when an Assume Role authentication is requested, but omitting them would raise an error.


-- 
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: github-unsubscribe@arrow.apache.org

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


[GitHub] [arrow] pitrou merged pull request #13455: ARROW-16864: [Python] Allow omitting S3 external_id and session_name with role_arn

Posted by GitBox <gi...@apache.org>.
pitrou merged PR #13455:
URL: https://github.com/apache/arrow/pull/13455


-- 
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: github-unsubscribe@arrow.apache.org

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


[GitHub] [arrow] pitrou commented on a diff in pull request #13455: ARROW-16864: [Python] Allow omitting S3 external_id and session_name with role_arn

Posted by GitBox <gi...@apache.org>.
pitrou commented on code in PR #13455:
URL: https://github.com/apache/arrow/pull/13455#discussion_r908810221


##########
cpp/src/arrow/filesystem/s3fs.cc:
##########
@@ -381,13 +381,21 @@ Result<S3Options> S3Options::FromUri(const std::string& uri_string,
 }
 
 bool S3Options::Equals(const S3Options& other) const {
+  const int64_t default_metadata_size = default_metadata ? default_metadata->size() : 0;
+  const bool default_metadata_equals =
+      default_metadata_size

Review Comment:
   Hmm, that is precisely the purpose of this check (allow a 0-size non-null metadata). Is there something wrong?



-- 
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: github-unsubscribe@arrow.apache.org

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


[GitHub] [arrow] lidavidm commented on a diff in pull request #13455: ARROW-16864: [Python] Allow omitting S3 external_id and session_name with role_arn

Posted by GitBox <gi...@apache.org>.
lidavidm commented on code in PR #13455:
URL: https://github.com/apache/arrow/pull/13455#discussion_r908825315


##########
cpp/src/arrow/filesystem/s3fs.cc:
##########
@@ -381,13 +381,21 @@ Result<S3Options> S3Options::FromUri(const std::string& uri_string,
 }
 
 bool S3Options::Equals(const S3Options& other) const {
+  const int64_t default_metadata_size = default_metadata ? default_metadata->size() : 0;
+  const bool default_metadata_equals =
+      default_metadata_size

Review Comment:
   No, I just read it wrong :) 



-- 
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: github-unsubscribe@arrow.apache.org

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


[GitHub] [arrow] github-actions[bot] commented on pull request #13455: ARROW-16864: [Python] Allow omitting S3 external_id and session_name with role_arn

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #13455:
URL: https://github.com/apache/arrow/pull/13455#issuecomment-1169014793

   https://issues.apache.org/jira/browse/ARROW-16864


-- 
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: github-unsubscribe@arrow.apache.org

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


[GitHub] [arrow] lidavidm commented on a diff in pull request #13455: ARROW-16864: [Python] Allow omitting S3 external_id and session_name with role_arn

Posted by GitBox <gi...@apache.org>.
lidavidm commented on code in PR #13455:
URL: https://github.com/apache/arrow/pull/13455#discussion_r908787463


##########
cpp/src/arrow/filesystem/s3fs.cc:
##########
@@ -381,13 +381,21 @@ Result<S3Options> S3Options::FromUri(const std::string& uri_string,
 }
 
 bool S3Options::Equals(const S3Options& other) const {
+  const int64_t default_metadata_size = default_metadata ? default_metadata->size() : 0;
+  const bool default_metadata_equals =
+      default_metadata_size

Review Comment:
   nit: is it possible to have a non-nullptr `default_metadata` that is of size 0? Should this check instead be `default_metadata`?



-- 
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: github-unsubscribe@arrow.apache.org

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