Port `object_store` integration tests, use github actions #2148

alamb · 2022-07-23T20:03:46Z

Which issue does this PR close?

re #2030

Rationale for this change

#2081 brought in the object_store_rs code, but did not include the test scripts for running object store tests against s3/azure/gcp simulators

What changes are included in this PR?

Adds github action based tests for running object store integration tests
Adds scripts + small object store configuration changes required to run them

It is based on my PR influxdata/object_store_rs#51 which I used to prototype the changes

I ported this using these commands:

$ git fetch git@github.com:alamb/object_store_rs.git alamb/test_gh_actions
$ git cherry-pick 25f4896
# and manually fixed some things

Are there any user-facing changes?

Not really

alamb · 2022-07-23T20:40:39Z

.github/workflows/object_store.yml

+      - object_store/**
+      - .github/**
+
+jobs:


This is more or less a direct port of https://github.com/influxdata/object_store_rs/blob/fe716c3/.circleci/config.yml

alamb · 2022-07-23T20:42:32Z

object_store/src/azure.rs

@@ -524,7 +553,27 @@ pub fn new_azure(

    let (is_emulator, storage_account_client) = if use_emulator {
        check_if_emulator_works()?;
-        (true, StorageAccountClient::new_emulator_default())
+        // Allow overriding defaults. Values taken from


This is required to specify a URL different than 127.0.0.1 -- the github actions "services" run with some actual dns name rather than on the local loopback (e.g. you need to specify http://auzure:10000 rather than http://127.0.0.1:10000)

codecov-commenter · 2022-07-23T20:46:22Z

Codecov Report

Merging #2148 (427107d) into master (73153fe) will decrease coverage by 0.00%.
The diff coverage is 40.00%.

@@            Coverage Diff             @@
##           master    #2148      +/-   ##
==========================================
- Coverage   82.82%   82.82%   -0.01%     
==========================================
  Files         237      237              
  Lines       61425    61428       +3     
==========================================
+ Hits        50874    50875       +1     
- Misses      10551    10553       +2

Impacted Files	Coverage Δ
object_store/src/azure.rs	`0.00% <ø> (ø)`
object_store/src/gcp.rs	`0.00% <0.00%> (ø)`
object_store/src/lib.rs	`84.42% <100.00%> (ø)`
...row/src/array/builder/string_dictionary_builder.rs	`90.64% <0.00%> (-0.72%)`	⬇️
parquet/src/encodings/encoding.rs	`93.43% <0.00%> (-0.20%)`	⬇️
arrow/src/datatypes/datatype.rs	`63.00% <0.00%> (+0.31%)`	⬆️
parquet_derive/src/parquet_field.rs	`66.21% <0.00%> (+0.45%)`	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us.

alamb · 2022-07-23T21:01:12Z

object_store/src/gcp.rs

@@ -575,6 +583,18 @@ mod test {
        service_account: String,
    }

+    impl GoogleCloudConfig {
+        fn build(self) -> Result<GoogleCloudStorage> {
+            // ignore HTTPS errors in tests so we can use fake-gcs server


Note that this is only done in the tests, whereas the original approach allowed it in production as well)

alamb · 2022-07-23T21:02:36Z

object_store/CONTRIBUTING.md

@@ -90,5 +90,24 @@ $ cargo test --features azure

 ### GCP

-We don't have a good story yet for testing the GCP integration locally. You will need to create a GCS bucket, a
-service account that has access to it, and use this to run the tests.
+To test the GCS integration, we use [Fake GCS Server](https://github.com/fsouza/fake-gcs-server)


Added documentation on how to run these integration tests

alamb · 2022-07-23T21:19:19Z

Here is an example of the new test running: https://github.com/apache/arrow-rs/runs/7483480192?check_suite_focus=true

alamb · 2022-07-23T21:54:52Z

cc @tustvold @wjones127

object_store/src/gcp.rs

Co-authored-by: Raphael Taylor-Davies <1781103+tustvold@users.noreply.github.com>

…tore_integration_test

alamb · 2022-07-25T10:47:53Z

object_store/src/lib.rs

@@ -695,8 +695,8 @@ mod tests {
    #[tokio::test]
    async fn test_list_lifetimes() {
        let store = memory::InMemory::new();
-        let stream = list_store(&store, "path").await.unwrap();
-        assert_eq!(stream.count().await, 0);
+        let mut stream = list_store(&store, "path").await.unwrap();


I am not sure why, but locally the stream.count() construction started erroring like this for me:

cd /Users/alamb/Software/arrow-rs2 && RUST_LOG=trace RUST_BACKTRACE=1 CARGO_TARGET_DIR=/Users/alamb/Software/df-target2 cargo test -p object_store --features=aws,azure,azure_test,gcp Compiling object_store v0.3.0 (/Users/alamb/Software/arrow-rs2/object_store) error[E0599]: `Pin<Box<dyn futures::Stream<Item = std::result::Result<ObjectMeta, Error>> + std::marker::Send>>` is not an iterator --> object_store/src/lib.rs:699:27 | 699 | assert_eq!(stream.count().await, 0); | ^^^^^ `Pin<Box<dyn futures::Stream<Item = std::result::Result<ObjectMeta, Error>> + std::marker::Send>>` is not an iterator | ::: /Users/alamb/.rustup/toolchains/stable-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/pin.rs:408:1 | 408 | pub struct Pin<P> { | ----------------- doesn't satisfy `_: Iterator` | ::: /Users/alamb/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-core-0.3.17/src/stream.rs:27:1 | 27 | pub trait Stream { | ---------------- doesn't satisfy `_: Iterator` | = note: the following trait bounds were not satisfied: `Pin<Box<dyn futures::Stream<Item = std::result::Result<ObjectMeta, Error>> + std::marker::Send>>: Iterator` which is required by `&mut Pin<Box<dyn futures::Stream<Item = std::result::Result<ObjectMeta, Error>> + std::marker::Send>>: Iterator` `dyn futures::Stream<Item = std::result::Result<ObjectMeta, Error>> + std::marker::Send: Iterator` which is required by `&mut dyn futures::Stream<Item = std::result::Result<ObjectMeta, Error>> + std::marker::Send: Iterator`

That sounds like the StreamExt is not in scope for some reason

Yeah, I tried a few ways to get that to work, but rustc gave me the same error and told me that StreamExt was unused ...

alamb · 2022-07-25T13:23:19Z

🤔 now the test https://github.com/apache/arrow-rs/runs/7499943986?check_suite_focus=true is failing with:

Run cargo check -p parquet_derive
    Checking syn v1.0.98
error: failed to write /github/home/target/debug/deps/rmetapeosRm/lib.rmeta: No space left on device (os error 28)
error: could not compile `syn` due to previous error
Error: Process completed with exit code [10](https://github.com/apache/arrow-rs/runs/7499943986?check_suite_focus=true#step:14:11)1.

I guess I will continue on the path laid out in #2149

alamb · 2022-07-25T13:23:57Z

However I don't see any reason to block merging this PR due to an unrelated issue, so merging

ursabot · 2022-07-25T13:31:12Z

Benchmark runs are scheduled for baseline = bcaa0b6 and contender = e68852c. e68852c is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Skipped ⚠️ Benchmarking of arrow-rs-commits is not supported on ec2-t3-xlarge-us-east-2] ec2-t3-xlarge-us-east-2
[Skipped ⚠️ Benchmarking of arrow-rs-commits is not supported on test-mac-arm] test-mac-arm
[Skipped ⚠️ Benchmarking of arrow-rs-commits is not supported on ursa-i9-9960x] ursa-i9-9960x
[Skipped ⚠️ Benchmarking of arrow-rs-commits is not supported on ursa-thinkcentre-m75q] ursa-thinkcentre-m75q
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

alamb added 3 commits July 23, 2022 14:53

Add github test skeleton

29e9049

Cleanups and fmt

fdff42d

Run on changes to object_store

5f17eb9

alamb changed the title ~~Run object_store_integration tests~~ Port object_store integration tests to arrow-rs / github actions: Jul 23, 2022

alamb changed the title ~~Port object_store integration tests to arrow-rs / github actions:~~ Port object_store integration tests to arrow-rs / github actions Jul 23, 2022

alamb mentioned this pull request Jul 23, 2022

Incorporate object_store into arrow-rs repository #2030

Closed

14 tasks

alamb added 4 commits July 23, 2022 15:13

Update name

d863a0d

Broken yaml?

ab251f9

Remove uneeded lint job

e32289f

Run only object store tests

b5e5f70

alamb commented Jul 23, 2022

View reviewed changes

alamb added 3 commits July 23, 2022 15:50

Add local gcp test instructions

bf1729e

Allow custom http client for gcs

56e46ca

remove unused error

d17fd07

alamb commented Jul 23, 2022

View reviewed changes

alamb marked this pull request as ready for review July 23, 2022 21:20

alamb mentioned this pull request Jul 23, 2022

Port Add stream upload (multi-part upload) #2147

Merged

Also run clippy

555c8f6

alamb mentioned this pull request Jul 23, 2022

Add github test skeleton (NOT FOR MERGING) influxdata/object_store_rs#51

Closed

alamb changed the title ~~Port object_store integration tests to arrow-rs / github actions~~ Port object_store integration tests, use github actions Jul 23, 2022

tustvold approved these changes Jul 24, 2022

View reviewed changes

object_store/src/gcp.rs Outdated Show resolved Hide resolved

wjones127 approved these changes Jul 24, 2022

View reviewed changes

Update object_store/src/gcp.rs

8823f30

Co-authored-by: Raphael Taylor-Davies <1781103+tustvold@users.noreply.github.com>

github-actions bot added the object-store Object Store Interface label Jul 25, 2022

Merge remote-tracking branch 'apache/master' into alamb/port_object_s…

310f3a5

…tore_integration_test

alamb added 2 commits July 25, 2022 06:37

rename more

3faf4c7

Fixup test

427107d

alamb commented Jul 25, 2022

View reviewed changes

alamb merged commit e68852c into apache:master Jul 25, 2022

alamb deleted the alamb/port_object_store_integration_test branch July 25, 2022 13:40

alamb mentioned this pull request Jul 25, 2022

Break parquet_derive and arrow_flight tests into their own workflows #2165

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port `object_store` integration tests, use github actions #2148

Port `object_store` integration tests, use github actions #2148

alamb commented Jul 23, 2022

alamb Jul 23, 2022

alamb Jul 23, 2022

codecov-commenter commented Jul 23, 2022 •

edited

Loading

alamb Jul 23, 2022

alamb Jul 23, 2022

alamb commented Jul 23, 2022

alamb commented Jul 23, 2022

alamb Jul 25, 2022

tustvold Jul 25, 2022

alamb Jul 25, 2022

alamb commented Jul 25, 2022

alamb commented Jul 25, 2022

ursabot commented Jul 25, 2022

Port object_store integration tests, use github actions #2148

Port object_store integration tests, use github actions #2148

Conversation

alamb commented Jul 23, 2022

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

alamb Jul 23, 2022

Choose a reason for hiding this comment

alamb Jul 23, 2022

Choose a reason for hiding this comment

codecov-commenter commented Jul 23, 2022 • edited Loading

Codecov Report

alamb Jul 23, 2022

Choose a reason for hiding this comment

alamb Jul 23, 2022

Choose a reason for hiding this comment

alamb commented Jul 23, 2022

alamb commented Jul 23, 2022

alamb Jul 25, 2022

Choose a reason for hiding this comment

tustvold Jul 25, 2022

Choose a reason for hiding this comment

alamb Jul 25, 2022

Choose a reason for hiding this comment

alamb commented Jul 25, 2022

alamb commented Jul 25, 2022

ursabot commented Jul 25, 2022

Port `object_store` integration tests, use github actions #2148

Port `object_store` integration tests, use github actions #2148

codecov-commenter commented Jul 23, 2022 •

edited

Loading