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/11/30 05:36:23 UTC

[GitHub] [arrow-rs] wjones127 opened a new pull request, #3236: fix(object_store,gcp): test copy_if_not_exist

wjones127 opened a new pull request, #3236:
URL: https://github.com/apache/arrow-rs/pull/3236

   # Which issue does this PR close?
   
   Closes #3235.
   
   # Rationale for this change
    
   The `copy_if_not_exist` function was not tested, and didn't pass the test when enabled. It needed to return an `AlreadyExists` error if the destination already existed.
   
   # What changes are included in this PR?
   
    * `copy_if_not_exist` returns `AlreadyExists` error when appropriate
    * GCP now tests `copy_if_not_exists` in integration tests
    * GCS copy commands send a `Content-Length` header now.
   
   # Are there any user-facing changes?
   
   
   <!--
   If there are user-facing changes then we may require documentation to be updated before approving the PR.
   -->
   
   <!---
   If there are any breaking changes to public APIs, please add the `breaking change` label.
   -->
   


-- 
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-rs] ursabot commented on pull request #3236: fix(object_store,gcp): test copy_if_not_exist

Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #3236:
URL: https://github.com/apache/arrow-rs/pull/3236#issuecomment-1335049589

   Benchmark runs are scheduled for baseline = 95cbca64e1dc30360304a1522f07c58dc661ef6b and contender = de3828cd71a17076147b07a796e4b97bc669648d. de3828cd71a17076147b07a796e4b97bc669648d is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Skipped :warning: Benchmarking of arrow-rs-commits is not supported on ec2-t3-xlarge-us-east-2] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/f0ae005b5f3a45cb86a916086048c112...9aef2f7fb74449419eba13a31143f872/)
   [Skipped :warning: Benchmarking of arrow-rs-commits is not supported on test-mac-arm] [test-mac-arm](https://conbench.ursa.dev/compare/runs/91a08586156d4e039c9c08a1dcc168fc...fa006b2f0cc44cf78e055f960f454f21/)
   [Skipped :warning: Benchmarking of arrow-rs-commits is not supported on ursa-i9-9960x] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/f23b08a521a24b389c679bf4fe5bc999...7c6e11ac2e284816ae7bffbe34daa9eb/)
   [Skipped :warning: Benchmarking of arrow-rs-commits is not supported on ursa-thinkcentre-m75q] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/2dd676e2c8054ffeb7b560fa9f01777d...f3215088e70b4116bb48145e36ad9a64/)
   Buildkite builds:
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
   test-mac-arm: Supported benchmark langs: C++, Python, R
   ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
   ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
   


-- 
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-rs] wjones127 commented on pull request #3236: fix(object_store,gcp): test copy_if_not_exist

Posted by GitBox <gi...@apache.org>.
wjones127 commented on PR #3236:
URL: https://github.com/apache/arrow-rs/pull/3236#issuecomment-1333152917

   I have run the integration tests for the gcs feature against my own GCS account and they all pass. ✅ 


-- 
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-rs] tustvold merged pull request #3236: fix(object_store,gcp): test copy_if_not_exist

Posted by GitBox <gi...@apache.org>.
tustvold merged PR #3236:
URL: https://github.com/apache/arrow-rs/pull/3236


-- 
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-rs] wjones127 commented on a diff in pull request #3236: fix(object_store,gcp): test copy_if_not_exist

Posted by GitBox <gi...@apache.org>.
wjones127 commented on code in PR #3236:
URL: https://github.com/apache/arrow-rs/pull/3236#discussion_r1037600571


##########
object_store/src/gcp/mod.rs:
##########
@@ -420,10 +430,25 @@ impl GoogleCloudStorageClient {
 
         builder
             .bearer_auth(token)
+            .header(header::CONTENT_LENGTH, 0)

Review Comment:
   I think you are right. I originally got an error report suggesting this was necessary, but haven't been able to reproduce that specific error. I'd be fine with striking this until I can get a proper reproduction.



-- 
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-rs] tustvold commented on a diff in pull request #3236: fix(object_store,gcp): test copy_if_not_exist

Posted by GitBox <gi...@apache.org>.
tustvold commented on code in PR #3236:
URL: https://github.com/apache/arrow-rs/pull/3236#discussion_r1037123180


##########
object_store/src/gcp/mod.rs:
##########
@@ -420,10 +430,25 @@ impl GoogleCloudStorageClient {
 
         builder
             .bearer_auth(token)
+            .header(header::CONTENT_LENGTH, 0)

Review Comment:
   At least when testing against GCP this change does not seem to be required in order for the tests to pass?



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