You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by GitBox <gi...@apache.org> on 2020/07/24 06:05:13 UTC
[GitHub] [hudi] xushiyan opened a new pull request #1871: [WIP] [HUDI-781] Introduce HoodieDataPrep for test preparation
xushiyan opened a new pull request #1871:
URL: https://github.com/apache/hudi/pull/1871
- Consolidate relevant util methods to `HoodieDataPrep`
- Make `HoodieDataPrep` the sole class for creating hoodie data/metadata files for testing
## Committer checklist
- [ ] Has a corresponding JIRA in PR title & commit
- [ ] Commit message is descriptive of the change
- [ ] CI is green
- [ ] Necessary doc changes done or have another open PR
- [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] yanghua commented on a change in pull request #1871: [HUDI-781] Introduce HoodieTestTable for test preparation
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1871:
URL: https://github.com/apache/hudi/pull/1871#discussion_r467827467
##########
File path: hudi-common/src/test/java/org/apache/hudi/common/testutils/HoodieTestUtils.java
##########
@@ -237,11 +229,12 @@ public static void createPendingCleanFiles(HoodieTableMetaClient metaClient, Str
public static void createCorruptedPendingCleanFiles(HoodieTableMetaClient metaClient, String commitTime) {
Arrays.asList(HoodieTimeline.makeRequestedCleanerFileName(commitTime),
- HoodieTimeline.makeInflightCleanerFileName(commitTime)).forEach(f -> {
+ HoodieTimeline.makeInflightCleanerFileName(commitTime))
+ .forEach(f -> {
Review comment:
Why do we need to break the `forEach`?
##########
File path: hudi-common/src/test/java/org/apache/hudi/common/testutils/HoodieTestUtils.java
##########
@@ -237,11 +229,12 @@ public static void createPendingCleanFiles(HoodieTableMetaClient metaClient, Str
public static void createCorruptedPendingCleanFiles(HoodieTableMetaClient metaClient, String commitTime) {
Arrays.asList(HoodieTimeline.makeRequestedCleanerFileName(commitTime),
- HoodieTimeline.makeInflightCleanerFileName(commitTime)).forEach(f -> {
+ HoodieTimeline.makeInflightCleanerFileName(commitTime))
+ .forEach(f -> {
FSDataOutputStream os = null;
try {
Path commitFile = new Path(
- metaClient.getBasePath() + "/" + HoodieTableMetaClient.METAFOLDER_NAME + "/" + f);
+ metaClient.getBasePath() + "/" + HoodieTableMetaClient.METAFOLDER_NAME + "/" + f);
Review comment:
Can we use `Paths.get()` to reduce link path parts via `/`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] xushiyan commented on pull request #1871: [WIP] [HUDI-781] Introduce HoodieDataPrep for test preparation
Posted by GitBox <gi...@apache.org>.
xushiyan commented on pull request #1871:
URL: https://github.com/apache/hudi/pull/1871#issuecomment-665437032
Make sense. Working on incorporating the feature of writing data to files.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] xushiyan commented on pull request #1871: [HUDI-781] Introduce HoodieTestTable for test preparation
Posted by GitBox <gi...@apache.org>.
xushiyan commented on pull request #1871:
URL: https://github.com/apache/hudi/pull/1871#issuecomment-671730074
@bvaradar no worries.. i can do another 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] xushiyan commented on a change in pull request #1871: [HUDI-781] Introduce HoodieTestTable for test preparation
Posted by GitBox <gi...@apache.org>.
xushiyan commented on a change in pull request #1871:
URL: https://github.com/apache/hudi/pull/1871#discussion_r467662611
##########
File path: hudi-common/src/test/java/org/apache/hudi/common/testutils/FilesTestUtils.java
##########
@@ -0,0 +1,104 @@
+/*
+ * 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.
+ */
+
+package org.apache.hudi.common.testutils;
+
+import org.apache.hudi.common.fs.FSUtils;
+import org.apache.hudi.common.model.HoodieFileFormat;
+import org.apache.hudi.common.model.IOType;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieTimeline;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.RandomAccessFile;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+
+public class FilesTestUtils {
+
+ private static void fakeMetaFile(String basePath, String instantTime, String suffix) throws IOException {
+ String parentPath = basePath + "/" + HoodieTableMetaClient.METAFOLDER_NAME;
+ new File(parentPath).mkdirs();
+ new File(parentPath + "/" + instantTime + suffix).createNewFile();
+ }
+
+ public static void fakeCommit(String basePath, String instantTime) throws IOException {
Review comment:
@yanghua how about `make`? since it is actually "making" stuff. e.g., legit marker files are indeed "made" by these APIs. Plus the follow-up work is to write actual data to these files, which make them real..
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] vinothchandar commented on pull request #1871: [HUDI-781] Introduce HoodieTestTable for test preparation
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on pull request #1871:
URL: https://github.com/apache/hudi/pull/1871#issuecomment-671685350
@yanghua @xushiyan can we please hold off on these refactoring PRs until we cut RCs please. we are trying to land the last bits. the rebase efforts from these, keep stretching the timelines.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] xushiyan commented on a change in pull request #1871: [HUDI-781] Introduce HoodieTestTable for test preparation
Posted by GitBox <gi...@apache.org>.
xushiyan commented on a change in pull request #1871:
URL: https://github.com/apache/hudi/pull/1871#discussion_r467662611
##########
File path: hudi-common/src/test/java/org/apache/hudi/common/testutils/FilesTestUtils.java
##########
@@ -0,0 +1,104 @@
+/*
+ * 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.
+ */
+
+package org.apache.hudi.common.testutils;
+
+import org.apache.hudi.common.fs.FSUtils;
+import org.apache.hudi.common.model.HoodieFileFormat;
+import org.apache.hudi.common.model.IOType;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieTimeline;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.RandomAccessFile;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+
+public class FilesTestUtils {
+
+ private static void fakeMetaFile(String basePath, String instantTime, String suffix) throws IOException {
+ String parentPath = basePath + "/" + HoodieTableMetaClient.METAFOLDER_NAME;
+ new File(parentPath).mkdirs();
+ new File(parentPath + "/" + instantTime + suffix).createNewFile();
+ }
+
+ public static void fakeCommit(String basePath, String instantTime) throws IOException {
Review comment:
@yanghua how about `create`? since it is actually "creating" stuff. e.g., legit marker files are indeed "created" by these APIs. Plus the follow-up work is to write actual data to these files, which make them real..
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] xushiyan commented on a change in pull request #1871: [HUDI-781] Introduce HoodieTestTable for test preparation
Posted by GitBox <gi...@apache.org>.
xushiyan commented on a change in pull request #1871:
URL: https://github.com/apache/hudi/pull/1871#discussion_r468021630
##########
File path: hudi-common/src/test/java/org/apache/hudi/common/testutils/HoodieTestUtils.java
##########
@@ -237,11 +229,12 @@ public static void createPendingCleanFiles(HoodieTableMetaClient metaClient, Str
public static void createCorruptedPendingCleanFiles(HoodieTableMetaClient metaClient, String commitTime) {
Arrays.asList(HoodieTimeline.makeRequestedCleanerFileName(commitTime),
- HoodieTimeline.makeInflightCleanerFileName(commitTime)).forEach(f -> {
+ HoodieTimeline.makeInflightCleanerFileName(commitTime))
+ .forEach(f -> {
Review comment:
intellij will force some weird indentation that breaks checkstyle rule...this is the easiest way I can find to prevent 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] xushiyan commented on a change in pull request #1871: [HUDI-781] Introduce HoodieTestTable for test preparation
Posted by GitBox <gi...@apache.org>.
xushiyan commented on a change in pull request #1871:
URL: https://github.com/apache/hudi/pull/1871#discussion_r467664319
##########
File path: hudi-common/src/test/java/org/apache/hudi/common/testutils/FilesTestUtils.java
##########
@@ -0,0 +1,104 @@
+/*
+ * 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.
+ */
+
+package org.apache.hudi.common.testutils;
+
+import org.apache.hudi.common.fs.FSUtils;
+import org.apache.hudi.common.model.HoodieFileFormat;
+import org.apache.hudi.common.model.IOType;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieTimeline;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.RandomAccessFile;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+
+public class FilesTestUtils {
Review comment:
ok i see your point. To follow the idea of `create` in another comment, how about `FileCreateUtils` ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] xushiyan commented on a change in pull request #1871: [HUDI-781] Introduce HoodieTestTable for test preparation
Posted by GitBox <gi...@apache.org>.
xushiyan commented on a change in pull request #1871:
URL: https://github.com/apache/hudi/pull/1871#discussion_r468032133
##########
File path: hudi-common/src/test/java/org/apache/hudi/common/testutils/HoodieTestUtils.java
##########
@@ -237,11 +229,12 @@ public static void createPendingCleanFiles(HoodieTableMetaClient metaClient, Str
public static void createCorruptedPendingCleanFiles(HoodieTableMetaClient metaClient, String commitTime) {
Arrays.asList(HoodieTimeline.makeRequestedCleanerFileName(commitTime),
- HoodieTimeline.makeInflightCleanerFileName(commitTime)).forEach(f -> {
+ HoodieTimeline.makeInflightCleanerFileName(commitTime))
+ .forEach(f -> {
FSDataOutputStream os = null;
try {
Path commitFile = new Path(
- metaClient.getBasePath() + "/" + HoodieTableMetaClient.METAFOLDER_NAME + "/" + f);
+ metaClient.getBasePath() + "/" + HoodieTableMetaClient.METAFOLDER_NAME + "/" + f);
Review comment:
sure, fixed 2 of this. Please note that, as this is from the original codebase, i didn't change all of this kind of usage in this file. I think it could be a good chance to change while moving the APIs to `HoodieTestTable`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] yanghua commented on a change in pull request #1871: [HUDI-781] Introduce HoodieTestTable for test preparation
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1871:
URL: https://github.com/apache/hudi/pull/1871#discussion_r467682591
##########
File path: hudi-common/src/test/java/org/apache/hudi/common/testutils/FilesTestUtils.java
##########
@@ -0,0 +1,104 @@
+/*
+ * 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.
+ */
+
+package org.apache.hudi.common.testutils;
+
+import org.apache.hudi.common.fs.FSUtils;
+import org.apache.hudi.common.model.HoodieFileFormat;
+import org.apache.hudi.common.model.IOType;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieTimeline;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.RandomAccessFile;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+
+public class FilesTestUtils {
Review comment:
sounds good.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] xushiyan commented on a change in pull request #1871: [HUDI-781] Introduce HoodieTestTable for test preparation
Posted by GitBox <gi...@apache.org>.
xushiyan commented on a change in pull request #1871:
URL: https://github.com/apache/hudi/pull/1871#discussion_r467661104
##########
File path: hudi-common/src/test/java/org/apache/hudi/common/testutils/HoodieTestUtils.java
##########
@@ -307,31 +299,26 @@ public static String createNewLogFile(FileSystem fs, String basePath, String par
return fileID;
}
- public static void createCompactionCommitFiles(FileSystem fs, String basePath, String... instantTimes)
- throws IOException {
- for (String instantTime : instantTimes) {
- boolean createFile = fs.createNewFile(new Path(basePath + "/" + HoodieTableMetaClient.METAFOLDER_NAME + "/"
- + HoodieTimeline.makeCommitFileName(instantTime)));
- if (!createFile) {
- throw new IOException("cannot create commit file for commit " + instantTime);
- }
- }
- }
-
public static void createCompactionRequest(HoodieTableMetaClient metaClient, String instant,
- List<Pair<String, FileSlice>> fileSliceList) throws IOException {
+ List<Pair<String, FileSlice>> fileSliceList) throws IOException {
Review comment:
Checkstyle did not complain about (or cover) it. The intellij of the new version starts to format this way. Guess it is due to missing a checkstyle rule for this, causing IDE format in its own way. Right now I've seen this style exist too in the codebase.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] vinothchandar commented on pull request #1871: [HUDI-781] Introduce HoodieTestTable for test preparation
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on pull request #1871:
URL: https://github.com/apache/hudi/pull/1871#issuecomment-675765424
No worries. @xushiyan could you reopen another 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] bvaradar commented on pull request #1871: [HUDI-781] Introduce HoodieTestTable for test preparation
Posted by GitBox <gi...@apache.org>.
bvaradar commented on pull request #1871:
URL: https://github.com/apache/hudi/pull/1871#issuecomment-671729188
@xushiyan @yanghua : This PR is causing lot of merge conflicts to a blocker PR which we needed to merge by tonight and I am unable to resolve conflicts in time. I am reverting this PR for now. Can you kindly re-merge this PR once 0.6. is cut.
THanks,
Balaji.V
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] yanghua commented on a change in pull request #1871: [HUDI-781] Introduce HoodieTestTable for test preparation
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1871:
URL: https://github.com/apache/hudi/pull/1871#discussion_r467530658
##########
File path: hudi-common/src/test/java/org/apache/hudi/common/testutils/FilesTestUtils.java
##########
@@ -0,0 +1,104 @@
+/*
+ * 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.
+ */
+
+package org.apache.hudi.common.testutils;
+
+import org.apache.hudi.common.fs.FSUtils;
+import org.apache.hudi.common.model.HoodieFileFormat;
+import org.apache.hudi.common.model.IOType;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieTimeline;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.RandomAccessFile;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+
+public class FilesTestUtils {
Review comment:
IMO, The name of this class may make readers confused? It seems to represent that it is a utility class about testing file function. WDYT about renaming it to `FileMockUtils`?
##########
File path: hudi-common/src/test/java/org/apache/hudi/common/testutils/HoodieTestUtils.java
##########
@@ -307,31 +299,26 @@ public static String createNewLogFile(FileSystem fs, String basePath, String par
return fileID;
}
- public static void createCompactionCommitFiles(FileSystem fs, String basePath, String... instantTimes)
- throws IOException {
- for (String instantTime : instantTimes) {
- boolean createFile = fs.createNewFile(new Path(basePath + "/" + HoodieTableMetaClient.METAFOLDER_NAME + "/"
- + HoodieTimeline.makeCommitFileName(instantTime)));
- if (!createFile) {
- throw new IOException("cannot create commit file for commit " + instantTime);
- }
- }
- }
-
public static void createCompactionRequest(HoodieTableMetaClient metaClient, String instant,
- List<Pair<String, FileSlice>> fileSliceList) throws IOException {
+ List<Pair<String, FileSlice>> fileSliceList) throws IOException {
Review comment:
Does this indentation style break the rule of the project?
##########
File path: hudi-common/src/test/java/org/apache/hudi/common/testutils/FilesTestUtils.java
##########
@@ -0,0 +1,104 @@
+/*
+ * 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.
+ */
+
+package org.apache.hudi.common.testutils;
+
+import org.apache.hudi.common.fs.FSUtils;
+import org.apache.hudi.common.model.HoodieFileFormat;
+import org.apache.hudi.common.model.IOType;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieTimeline;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.RandomAccessFile;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+
+public class FilesTestUtils {
+
+ private static void fakeMetaFile(String basePath, String instantTime, String suffix) throws IOException {
+ String parentPath = basePath + "/" + HoodieTableMetaClient.METAFOLDER_NAME;
+ new File(parentPath).mkdirs();
+ new File(parentPath + "/" + instantTime + suffix).createNewFile();
+ }
+
+ public static void fakeCommit(String basePath, String instantTime) throws IOException {
Review comment:
IMO, `mock` is a better naming than `fake`. `mock` is a more generic term in test field. Just a thought.
##########
File path: hudi-common/src/test/java/org/apache/hudi/common/testutils/FilesTestUtils.java
##########
@@ -0,0 +1,104 @@
+/*
+ * 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.
+ */
+
+package org.apache.hudi.common.testutils;
+
+import org.apache.hudi.common.fs.FSUtils;
+import org.apache.hudi.common.model.HoodieFileFormat;
+import org.apache.hudi.common.model.IOType;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieTimeline;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.RandomAccessFile;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+
+public class FilesTestUtils {
+
+ private static void fakeMetaFile(String basePath, String instantTime, String suffix) throws IOException {
+ String parentPath = basePath + "/" + HoodieTableMetaClient.METAFOLDER_NAME;
+ new File(parentPath).mkdirs();
+ new File(parentPath + "/" + instantTime + suffix).createNewFile();
+ }
+
+ public static void fakeCommit(String basePath, String instantTime) throws IOException {
+ fakeMetaFile(basePath, instantTime, HoodieTimeline.COMMIT_EXTENSION);
+ }
+
+ public static void fakeRequestedCommit(String basePath, String instantTime) throws IOException {
+ fakeMetaFile(basePath, instantTime, HoodieTimeline.REQUESTED_COMMIT_EXTENSION);
+ }
+
+ public static void fakeInflightCommit(String basePath, String instantTime) throws IOException {
+ fakeMetaFile(basePath, instantTime, HoodieTimeline.INFLIGHT_COMMIT_EXTENSION);
+ }
+
+ public static void fakeDeltaCommit(String basePath, String instantTime) throws IOException {
+ fakeMetaFile(basePath, instantTime, HoodieTimeline.DELTA_COMMIT_EXTENSION);
+ }
+
+ public static void fakeRequestedDeltaCommit(String basePath, String instantTime) throws IOException {
+ fakeMetaFile(basePath, instantTime, HoodieTimeline.REQUESTED_DELTA_COMMIT_EXTENSION);
+ }
+
+ public static void fakeInflightDeltaCommit(String basePath, String instantTime) throws IOException {
+ fakeMetaFile(basePath, instantTime, HoodieTimeline.INFLIGHT_DELTA_COMMIT_EXTENSION);
+ }
+
+ public static void fakeDataFile(String basePath, String partitionPath, String instantTime, String fileId)
+ throws Exception {
+ fakeDataFile(basePath, partitionPath, instantTime, fileId, 0);
+ }
+
+ public static void fakeDataFile(String basePath, String partitionPath, String instantTime, String fileId, long length)
+ throws Exception {
+ String parentPath = String.format("%s/%s", basePath, partitionPath);
+ new File(parentPath).mkdirs();
Review comment:
Using `Files.createFile(...)` may be better?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] bvaradar commented on pull request #1871: [HUDI-781] Introduce HoodieTestTable for test preparation
Posted by GitBox <gi...@apache.org>.
bvaradar commented on pull request #1871:
URL: https://github.com/apache/hudi/pull/1871#issuecomment-671730869
Thanks @xushiyan
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] yanghua commented on pull request #1871: [HUDI-781] Introduce HoodieTestTable for test preparation
Posted by GitBox <gi...@apache.org>.
yanghua commented on pull request #1871:
URL: https://github.com/apache/hudi/pull/1871#issuecomment-671742888
> @xushiyan @yanghua : This PR is causing lot of merge conflicts to a blocker PR which we needed to merge by tonight and I am unable to resolve conflicts in time. I am reverting this PR for now. Can you kindly re-merge this PR once 0.6. is cut.
>
> THanks,
> Balaji.V
Ok, sorry for interrupting the release plan.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] xushiyan commented on pull request #1871: [HUDI-781] Introduce HoodieTestTable for test preparation
Posted by GitBox <gi...@apache.org>.
xushiyan commented on pull request #1871:
URL: https://github.com/apache/hudi/pull/1871#issuecomment-675767130
> No worries. @xushiyan could you reopen another one?
Yea @vinothchandar i'm about to submitting another one. but the major change in `TestCleaner` is unresolvably conflicted. got to rework on it. so probably get it out tmr or so
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] xushiyan commented on a change in pull request #1871: [HUDI-781] Introduce HoodieTestTable for test preparation
Posted by GitBox <gi...@apache.org>.
xushiyan commented on a change in pull request #1871:
URL: https://github.com/apache/hudi/pull/1871#discussion_r467367284
##########
File path: hudi-client/src/test/java/org/apache/hudi/testutils/HoodieClientTestUtils.java
##########
@@ -227,6 +176,8 @@ public static SparkConf getSparkConfForTest(String appName) {
/**
* Find total basefiles for passed in paths.
+ *
+ * @deprecated Use {@link HoodieTestTable} instead.
Review comment:
mark deprecated to prevent new usage; also as a reminder to migrate in future PRs. Similar to other deprecated marks.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] vinothchandar commented on pull request #1871: [WIP] [HUDI-781] Introduce HoodieDataPrep for test preparation
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on pull request #1871:
URL: https://github.com/apache/hudi/pull/1871#issuecomment-665363288
On the DataPrep I was wondering through,
if we should call it `HoodieTestTable` or something, since it's just creating some files, not really writing data. Also our code will probably generate 1,2 commits in most places. So should it include helpers like fakeTestTable.generateTwoCommits() etc. Idea here is to make test shorter. I am fine to just begin by standardizing though.
We should probably have another `HoodieTestTableWithData` which actually generates test table using regular writeClient APIs, does sanity asserts. and actually writes data into these files
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] xushiyan commented on a change in pull request #1871: [HUDI-781] Introduce HoodieTestTable for test preparation
Posted by GitBox <gi...@apache.org>.
xushiyan commented on a change in pull request #1871:
URL: https://github.com/apache/hudi/pull/1871#discussion_r467669290
##########
File path: hudi-common/src/test/java/org/apache/hudi/common/testutils/HoodieTestUtils.java
##########
@@ -307,31 +299,26 @@ public static String createNewLogFile(FileSystem fs, String basePath, String par
return fileID;
}
- public static void createCompactionCommitFiles(FileSystem fs, String basePath, String... instantTimes)
- throws IOException {
- for (String instantTime : instantTimes) {
- boolean createFile = fs.createNewFile(new Path(basePath + "/" + HoodieTableMetaClient.METAFOLDER_NAME + "/"
- + HoodieTimeline.makeCommitFileName(instantTime)));
- if (!createFile) {
- throw new IOException("cannot create commit file for commit " + instantTime);
- }
- }
- }
-
public static void createCompactionRequest(HoodieTableMetaClient metaClient, String instant,
- List<Pair<String, FileSlice>> fileSliceList) throws IOException {
+ List<Pair<String, FileSlice>> fileSliceList) throws IOException {
Review comment:
I see the issue is that we did not set `forceStrictCondition` to `true`, that's why intellij can align this and make the indentation longer than what the rule sets.
https://checkstyle.sourceforge.io/config_misc.html#Indentation_Properties
@yanghua Do you agree we should set it to `true`? This can be done in a separate 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] xushiyan commented on a change in pull request #1871: [HUDI-781] Introduce HoodieTestTable for test preparation
Posted by GitBox <gi...@apache.org>.
xushiyan commented on a change in pull request #1871:
URL: https://github.com/apache/hudi/pull/1871#discussion_r467661774
##########
File path: hudi-common/src/test/java/org/apache/hudi/common/testutils/FilesTestUtils.java
##########
@@ -0,0 +1,104 @@
+/*
+ * 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.
+ */
+
+package org.apache.hudi.common.testutils;
+
+import org.apache.hudi.common.fs.FSUtils;
+import org.apache.hudi.common.model.HoodieFileFormat;
+import org.apache.hudi.common.model.IOType;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieTimeline;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.RandomAccessFile;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+
+public class FilesTestUtils {
+
+ private static void fakeMetaFile(String basePath, String instantTime, String suffix) throws IOException {
+ String parentPath = basePath + "/" + HoodieTableMetaClient.METAFOLDER_NAME;
+ new File(parentPath).mkdirs();
+ new File(parentPath + "/" + instantTime + suffix).createNewFile();
+ }
+
+ public static void fakeCommit(String basePath, String instantTime) throws IOException {
Review comment:
hmm i'm a bit hesitated about using `mock` as this is commonly used by `Mockito`. it could imply mock object is used here, while we're actually creating files..
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] yanghua merged pull request #1871: [HUDI-781] Introduce HoodieTestTable for test preparation
Posted by GitBox <gi...@apache.org>.
yanghua merged pull request #1871:
URL: https://github.com/apache/hudi/pull/1871
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] yanghua commented on a change in pull request #1871: [HUDI-781] Introduce HoodieTestTable for test preparation
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1871:
URL: https://github.com/apache/hudi/pull/1871#discussion_r467682429
##########
File path: hudi-common/src/test/java/org/apache/hudi/common/testutils/FilesTestUtils.java
##########
@@ -0,0 +1,104 @@
+/*
+ * 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.
+ */
+
+package org.apache.hudi.common.testutils;
+
+import org.apache.hudi.common.fs.FSUtils;
+import org.apache.hudi.common.model.HoodieFileFormat;
+import org.apache.hudi.common.model.IOType;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieTimeline;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.RandomAccessFile;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+
+public class FilesTestUtils {
+
+ private static void fakeMetaFile(String basePath, String instantTime, String suffix) throws IOException {
+ String parentPath = basePath + "/" + HoodieTableMetaClient.METAFOLDER_NAME;
+ new File(parentPath).mkdirs();
+ new File(parentPath + "/" + instantTime + suffix).createNewFile();
+ }
+
+ public static void fakeCommit(String basePath, String instantTime) throws IOException {
Review comment:
Ok, sg.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] xushiyan commented on a change in pull request #1871: [HUDI-781] Introduce HoodieTestTable for test preparation
Posted by GitBox <gi...@apache.org>.
xushiyan commented on a change in pull request #1871:
URL: https://github.com/apache/hudi/pull/1871#discussion_r468279092
##########
File path: hudi-common/src/test/java/org/apache/hudi/common/testutils/HoodieTestUtils.java
##########
@@ -237,11 +229,12 @@ public static void createPendingCleanFiles(HoodieTableMetaClient metaClient, Str
public static void createCorruptedPendingCleanFiles(HoodieTableMetaClient metaClient, String commitTime) {
Arrays.asList(HoodieTimeline.makeRequestedCleanerFileName(commitTime),
- HoodieTimeline.makeInflightCleanerFileName(commitTime)).forEach(f -> {
+ HoodieTimeline.makeInflightCleanerFileName(commitTime))
+ .forEach(f -> {
FSDataOutputStream os = null;
try {
Path commitFile = new Path(
- metaClient.getBasePath() + "/" + HoodieTableMetaClient.METAFOLDER_NAME + "/" + f);
+ metaClient.getBasePath() + "/" + HoodieTableMetaClient.METAFOLDER_NAME + "/" + f);
Review comment:
ok @yanghua , understood. I agree that the work should focus on the task. I didn't intend to change these; intellij did it for me. i should have disabled the auto-formatting. will keep this sort of diffs out from now on. 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] yanghua commented on a change in pull request #1871: [HUDI-781] Introduce HoodieTestTable for test preparation
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1871:
URL: https://github.com/apache/hudi/pull/1871#discussion_r468276194
##########
File path: hudi-common/src/test/java/org/apache/hudi/common/testutils/HoodieTestUtils.java
##########
@@ -237,11 +229,12 @@ public static void createPendingCleanFiles(HoodieTableMetaClient metaClient, Str
public static void createCorruptedPendingCleanFiles(HoodieTableMetaClient metaClient, String commitTime) {
Arrays.asList(HoodieTimeline.makeRequestedCleanerFileName(commitTime),
- HoodieTimeline.makeInflightCleanerFileName(commitTime)).forEach(f -> {
+ HoodieTimeline.makeInflightCleanerFileName(commitTime))
+ .forEach(f -> {
FSDataOutputStream os = null;
try {
Path commitFile = new Path(
- metaClient.getBasePath() + "/" + HoodieTableMetaClient.METAFOLDER_NAME + "/" + f);
+ metaClient.getBasePath() + "/" + HoodieTableMetaClient.METAFOLDER_NAME + "/" + f);
Review comment:
Ok, I noticed this because I saw the indent has been changed. Actually, I suggested that we can focus on the core work about the PR want to do. Do not touch something else e.g. indent and code style. Of cause, I should not ask you to change this.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] xushiyan commented on pull request #1871: [HUDI-781] Introduce HoodieTestTable for test preparation
Posted by GitBox <gi...@apache.org>.
xushiyan commented on pull request #1871:
URL: https://github.com/apache/hudi/pull/1871#issuecomment-671721410
@vinothchandar sorry i'll make the PRs in draft state until the cut.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] yanghua commented on a change in pull request #1871: [HUDI-781] Introduce HoodieTestTable for test preparation
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1871:
URL: https://github.com/apache/hudi/pull/1871#discussion_r467683562
##########
File path: hudi-common/src/test/java/org/apache/hudi/common/testutils/HoodieTestUtils.java
##########
@@ -307,31 +299,26 @@ public static String createNewLogFile(FileSystem fs, String basePath, String par
return fileID;
}
- public static void createCompactionCommitFiles(FileSystem fs, String basePath, String... instantTimes)
- throws IOException {
- for (String instantTime : instantTimes) {
- boolean createFile = fs.createNewFile(new Path(basePath + "/" + HoodieTableMetaClient.METAFOLDER_NAME + "/"
- + HoodieTimeline.makeCommitFileName(instantTime)));
- if (!createFile) {
- throw new IOException("cannot create commit file for commit " + instantTime);
- }
- }
- }
-
public static void createCompactionRequest(HoodieTableMetaClient metaClient, String instant,
- List<Pair<String, FileSlice>> fileSliceList) throws IOException {
+ List<Pair<String, FileSlice>> fileSliceList) throws IOException {
Review comment:
Let @vinothchandar chime in. I knew that @vinothchandar 's major concern is that whether the rule of code style support automatically formatting or not.
Here I will ignore these issues.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] xushiyan commented on a change in pull request #1871: [WIP] [HUDI-781] Introduce HoodieTestTable for test preparation
Posted by GitBox <gi...@apache.org>.
xushiyan commented on a change in pull request #1871:
URL: https://github.com/apache/hudi/pull/1871#discussion_r467367212
##########
File path: hudi-client/src/test/java/org/apache/hudi/testutils/HoodieClientTestUtils.java
##########
@@ -74,57 +74,6 @@
public class HoodieClientTestUtils {
Review comment:
deleted APIs that turn out be to useful moved to org.apache.hudi.common.testutils.FilesTestUtils
##########
File path: hudi-common/src/test/java/org/apache/hudi/common/testutils/HoodieTestUtils.java
##########
@@ -99,7 +99,6 @@
*/
public class HoodieTestUtils {
- public static final String TEST_EXTENSION = ".test";
Review comment:
not used
##########
File path: hudi-client/src/test/java/org/apache/hudi/testutils/HoodieClientTestUtils.java
##########
@@ -227,6 +176,8 @@ public static SparkConf getSparkConfForTest(String appName) {
/**
* Find total basefiles for passed in paths.
+ *
+ * @deprecated Use {@link HoodieTestTable} instead.
Review comment:
mark deprecated to prevent new usage; also as a reminder to migrate in future PRs. Similar to other deprecated marks.
##########
File path: hudi-common/src/test/java/org/apache/hudi/common/testutils/HoodieTestUtils.java
##########
@@ -279,8 +268,8 @@ public static String createDataFile(String basePath, String partitionPath, Strin
return fileID;
}
- public static String createDataFileFixLength(String basePath, String partitionPath, String instantTime, String fileID,
- long length) throws IOException {
+ private static String createDataFileFixLength(String basePath, String partitionPath, String instantTime, String fileID,
Review comment:
mark private to prevent new usage. to be replaced with HoodieTestTable, too.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org