You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "abyssnlp (via GitHub)" <gi...@apache.org> on 2024/08/13 20:27:11 UTC
[PR] test: add integration test with minio [hudi-rs]
abyssnlp opened a new pull request, #112:
URL: https://github.com/apache/hudi-rs/pull/112
## Description
Adds integration tests with s3-compatible MinIO.
- `docker/docker-compose.yaml` spins up the minio container
- `docker/copy_tables.sh` creates the bucket; unzips, copies and creates the tables in the minio store
- The environment variables for the integration test are supplied via `integration_test/env.toml`
- Adds running integration tests on pull requests via `ci.yml`
### Notes:
- We can extract the logic to run the integration tests in the `Makefile` or as a separate script
- The integration test takes around ~3 minutes to complete
- The integration test is only for the `hudi-rs` rust support via datafusion
This closes #81 .
## How are the changes test-covered
- [ ] N/A
- [x] Automated tests (unit and/or integration tests)
- [ ] Manual tests
- [ ] Details are described below
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test: add integration test with minio [hudi-rs]
Posted by "abyssnlp (via GitHub)" <gi...@apache.org>.
abyssnlp commented on PR #112:
URL: https://github.com/apache/hudi-rs/pull/112#issuecomment-2295272413
@xushiyan Addressed all comments in [a28debc](https://github.com/apache/hudi-rs/pull/112/commits/a28debc252bae3d9c8334d5a9aef4a35b66aa5a7).
I've reused the `TestTable` enum but I had to add an extra method to get the s3 path as we unzip and load the tables into minio for the integration tests so we don't need to extract them.
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test: add integration test with minio [hudi-rs]
Posted by "xushiyan (via GitHub)" <gi...@apache.org>.
xushiyan closed pull request #112: test: add integration test with minio
URL: https://github.com/apache/hudi-rs/pull/112
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test: add integration test with minio [hudi-rs]
Posted by "xushiyan (via GitHub)" <gi...@apache.org>.
xushiyan commented on PR #112:
URL: https://github.com/apache/hudi-rs/pull/112#issuecomment-2549496004
re-worked in https://github.com/apache/hudi-rs/pull/226
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test: add integration test with minio [hudi-rs]
Posted by "abyssnlp (via GitHub)" <gi...@apache.org>.
abyssnlp commented on PR #112:
URL: https://github.com/apache/hudi-rs/pull/112#issuecomment-2341961837
Hi @xushiyan,
I've been busier than usual so couldn't get around to this earlier. I've addressed your comments in https://github.com/apache/hudi-rs/pull/112/commits/5a747fd1dc45fccf85cfdcad6fd8ae5068fc8993.
- The integration tests now run in a container
- I'll add a test for python as well later
Let me know what you think.
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test: add integration test with minio [hudi-rs]
Posted by "xushiyan (via GitHub)" <gi...@apache.org>.
xushiyan commented on code in PR #112:
URL: https://github.com/apache/hudi-rs/pull/112#discussion_r1720820708
##########
integration_test/tests/hudi_test.rs:
##########
@@ -0,0 +1,276 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+use std::{path::Path, sync::Arc};
+
+use datafusion::{
+ arrow::{
+ array::{self, ArrowPrimitiveType, AsArray},
+ datatypes::{DataType, Field, Int64Type, Schema, TimeUnit},
+ },
+ prelude::*,
+ scalar::ScalarValue,
+};
+use hudi::HudiDataSource;
+use strum::IntoEnumIterator;
+use strum_macros::{AsRefStr, EnumIter, EnumString};
+use url::Url;
+
+#[derive(PartialEq, Debug)]
+struct Record {
+ id: i32,
+ name: String,
+ is_active: bool,
+}
+
+#[derive(Debug, EnumString, EnumIter, AsRefStr, PartialEq)]
+#[strum(serialize_all = "snake_case")]
+pub enum IntegrationTestTable {
+ V6ComplexkeygenHivestyle,
+ V6Empty,
+ V6Nonpartitioned,
+ V6SimplekeygenHivestyleNoMetafields,
+ V6SimplekeygenNonhivestyle,
+ V6SimplekeygenNonhivestyleOverwritetable, // 1, 21
Review Comment:
what does 1, 21 mean?
##########
.github/workflows/pr.yml:
##########
@@ -18,7 +18,7 @@
name: PR
on:
- pull_request:
+ pull_request_target:
Review Comment:
let's revert this. there is asf policy against using this. we can ignore the labeler issue for now.
##########
integration_test/tests/hudi_test.rs:
##########
@@ -0,0 +1,276 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+use std::{path::Path, sync::Arc};
+
+use datafusion::{
+ arrow::{
+ array::{self, ArrowPrimitiveType, AsArray},
+ datatypes::{DataType, Field, Int64Type, Schema, TimeUnit},
+ },
+ prelude::*,
+ scalar::ScalarValue,
+};
+use hudi::HudiDataSource;
+use strum::IntoEnumIterator;
+use strum_macros::{AsRefStr, EnumIter, EnumString};
+use url::Url;
+
+#[derive(PartialEq, Debug)]
+struct Record {
+ id: i32,
+ name: String,
+ is_active: bool,
+}
+
+#[derive(Debug, EnumString, EnumIter, AsRefStr, PartialEq)]
+#[strum(serialize_all = "snake_case")]
+pub enum IntegrationTestTable {
+ V6ComplexkeygenHivestyle,
+ V6Empty,
+ V6Nonpartitioned,
+ V6SimplekeygenHivestyleNoMetafields,
+ V6SimplekeygenNonhivestyle,
+ V6SimplekeygenNonhivestyleOverwritetable, // 1, 21
+ V6TimebasedkeygenNonhivestyle,
+}
+
+impl IntegrationTestTable {
+ pub fn path(&self) -> String {
+ let bucket = std::env::var("INTEGRATION_TEST_S3_BUCKET")
+ .expect("INTEGRATION_TEST_S3_BUCKET not set");
+ let data_path = Path::new(format!("s3://{}", bucket).as_str()).join(self.as_ref());
+ data_path.to_str().unwrap().to_string()
+ }
+
+ pub fn url(&self) -> Url {
+ let path = self.path();
+ Url::parse(&path).unwrap()
+ }
+}
+
+#[test]
+fn test_table_path() {
Review Comment:
in integ test we can leave out this path testing
##########
integration_test/tests/hudi_test.rs:
##########
@@ -0,0 +1,276 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+use std::{path::Path, sync::Arc};
+
+use datafusion::{
+ arrow::{
+ array::{self, ArrowPrimitiveType, AsArray},
+ datatypes::{DataType, Field, Int64Type, Schema, TimeUnit},
+ },
+ prelude::*,
+ scalar::ScalarValue,
+};
+use hudi::HudiDataSource;
+use strum::IntoEnumIterator;
+use strum_macros::{AsRefStr, EnumIter, EnumString};
+use url::Url;
+
+#[derive(PartialEq, Debug)]
+struct Record {
+ id: i32,
+ name: String,
+ is_active: bool,
+}
+
+#[derive(Debug, EnumString, EnumIter, AsRefStr, PartialEq)]
+#[strum(serialize_all = "snake_case")]
+pub enum IntegrationTestTable {
Review Comment:
can you reuse the code from crates/tests/src/lib.rs:39
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test: add integration test with minio [hudi-rs]
Posted by "abyssnlp (via GitHub)" <gi...@apache.org>.
abyssnlp commented on PR #112:
URL: https://github.com/apache/hudi-rs/pull/112#issuecomment-2290678791
@xushiyan The CI is failing due to this error:
`dtolnay/rust-toolchain@1.75.0 is not allowed to be used in apache/hudi-rs.` I used this action to setup the rust toolchain. Is there a way to whitelist this action or should I use an alternative?
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test: add integration test with minio [hudi-rs]
Posted by "abyssnlp (via GitHub)" <gi...@apache.org>.
abyssnlp commented on PR #112:
URL: https://github.com/apache/hudi-rs/pull/112#issuecomment-2290921344
@xushiyan All tests (including integration) are green. The labeler is failing due to [permissions](https://github.com/actions/labeler#permissions).
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test: add integration test with minio [hudi-rs]
Posted by "xushiyan (via GitHub)" <gi...@apache.org>.
xushiyan commented on PR #112:
URL: https://github.com/apache/hudi-rs/pull/112#issuecomment-2290467648
@abyssnlp don't think the ci job is triggered. probably format issue.
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test: add integration test with minio [hudi-rs]
Posted by "xushiyan (via GitHub)" <gi...@apache.org>.
xushiyan commented on PR #112:
URL: https://github.com/apache/hudi-rs/pull/112#issuecomment-2341606877
@abyssnlp are you planning to resume this work?
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test: add integration test with minio [hudi-rs]
Posted by "xushiyan (via GitHub)" <gi...@apache.org>.
xushiyan commented on PR #112:
URL: https://github.com/apache/hudi-rs/pull/112#issuecomment-2351413292
Ok, due to time constraint, I'll push this to the next release.
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test: add integration test with minio [hudi-rs]
Posted by "abyssnlp (via GitHub)" <gi...@apache.org>.
abyssnlp commented on PR #112:
URL: https://github.com/apache/hudi-rs/pull/112#issuecomment-2290729897
I've removed the external action since GHA runner has `rustup` already.
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test: add integration test with minio [hudi-rs]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #112:
URL: https://github.com/apache/hudi-rs/pull/112#issuecomment-2287193522
## [Codecov](https://app.codecov.io/gh/apache/hudi-rs/pull/112?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `0%` with `4 lines` in your changes missing coverage. Please review.
> Project coverage is 86.93%. Comparing base [(`3359e10`)](https://app.codecov.io/gh/apache/hudi-rs/commit/3359e108b8806c50c008072ef6afbcca86b2e4da?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`518a8f7`)](https://app.codecov.io/gh/apache/hudi-rs/commit/518a8f72a9647f82afd38ec4f30e801c1967ce69?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
| [Files](https://app.codecov.io/gh/apache/hudi-rs/pull/112?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [integration\_test/tests/hudi\_test.rs](https://app.codecov.io/gh/apache/hudi-rs/pull/112?src=pr&el=tree&filepath=integration_test%2Ftests%2Fhudi_test.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aW50ZWdyYXRpb25fdGVzdC90ZXN0cy9odWRpX3Rlc3QucnM=) | 0.00% | [4 Missing :warning: ](https://app.codecov.io/gh/apache/hudi-rs/pull/112?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #112 +/- ##
==========================================
- Coverage 87.42% 86.93% -0.50%
==========================================
Files 14 15 +1
Lines 700 704 +4
==========================================
Hits 612 612
- Misses 88 92 +4
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/hudi-rs/pull/112?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test: add integration test with minio [hudi-rs]
Posted by "xushiyan (via GitHub)" <gi...@apache.org>.
xushiyan commented on code in PR #112:
URL: https://github.com/apache/hudi-rs/pull/112#discussion_r1723556543
##########
docker/copy_tables.sh:
##########
@@ -0,0 +1,34 @@
+#!/bin/bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+set -eux
+
+echo ">>> Creating Minio store"
+mc config host add store http://minio:9000 $MINIO_ACCESS_KEY $MINIO_SECRET_KEY
+
+echo ">>> Creating bucket"
+mc mb store/$MINIO_TEST_BUCKET
+
+echo ">>> Unzipping tables"
+find /opt/data -name "*.zip" -exec unzip {} -d /tmp/data/ \;
+
+echo ">>> Copying tables to Minio"
+find /tmp/data -type d -mindepth 1 -maxdepth 1 -exec mc cp --recursive {} store/$MINIO_TEST_BUCKET \;
+
+echo ">>> Listing uploaded tables"
+mc ls store/$MINIO_TEST_BUCKET
Review Comment:
can this script be incorporated into Dockerfile? it's just a few commands after all. we want to keep things more consolidated
##########
integration_test/src/lib.rs:
##########
@@ -0,0 +1,18 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
Review Comment:
why create a lib ? it's just to run an application so bin works better? we don't expect another crate to depend on integ test
##########
integration_test/env.toml:
##########
@@ -0,0 +1,22 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+[env]
+AWS_ACCESS_KEY_ID="minio"
+AWS_SECRET_ACCESS_KEY="minio123"
+AWS_ENDPOINT="http://localhost:9000"
+INTEGRATION_TEST_S3_BUCKET="test-bucket"
Review Comment:
is this still needed given there is `s3.env` ? can those 2 be consolidated?
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org