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 2021/02/04 06:18:47 UTC
[GitHub] [hudi] MyLanPangzi opened a new pull request #2526: [HUDI-1420] HoodieTableMetaClient.getMarkerFolderPath works incorrectly on windows client with hdfs server for wrong file seperator
MyLanPangzi opened a new pull request #2526:
URL: https://github.com/apache/hudi/pull/2526
FIX https://issues.apache.org/jira/browse/HUDI-1420
----------------------------------------------------------------
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] wangxianghu merged pull request #2526: [HUDI-1420] HoodieTableMetaClient.getMarkerFolderPath works incorrectly on windows client with hdfs server for wrong file seperator
Posted by GitBox <gi...@apache.org>.
wangxianghu merged pull request #2526:
URL: https://github.com/apache/hudi/pull/2526
----------------------------------------------------------------
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] MyLanPangzi closed pull request #2526: [HUDI-1420] HoodieTableMetaClient.getMarkerFolderPath works incorrectly on windows client with hdfs server for wrong file seperator
Posted by GitBox <gi...@apache.org>.
MyLanPangzi closed pull request #2526:
URL: https://github.com/apache/hudi/pull/2526
----------------------------------------------------------------
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] wangxianghu commented on pull request #2526: [HUDI-1420] HoodieTableMetaClient.getMarkerFolderPath works incorrectly on windows client with hdfs server for wrong file seperator
Posted by GitBox <gi...@apache.org>.
wangxianghu commented on pull request #2526:
URL: https://github.com/apache/hudi/pull/2526#issuecomment-773192292
@yanghua any other concern ?
----------------------------------------------------------------
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] MyLanPangzi commented on a change in pull request #2526: [HUDI-1420] HoodieTableMetaClient.getMarkerFolderPath works incorrectly on windows client with hdfs server for wrong file seperator
Posted by GitBox <gi...@apache.org>.
MyLanPangzi commented on a change in pull request #2526:
URL: https://github.com/apache/hudi/pull/2526#discussion_r569972148
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableMetaClient.java
##########
@@ -118,7 +120,7 @@ public HoodieTableMetaClient(Configuration conf, String basePath, boolean loadAc
ConsistencyGuardConfig consistencyGuardConfig, Option<TimelineLayoutVersion> layoutVersion,
String payloadClassName) {
LOG.info("Loading HoodieTableMetaClient from " + basePath);
- this.basePath = basePath;
+ this.basePath = FilenameUtils.separatorsToUnix(basePath);
Review comment:
fixed and then ？
----------------------------------------------------------------
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] wangxianghu commented on pull request #2526: [HUDI-1420] HoodieTableMetaClient.getMarkerFolderPath works incorrectly on windows client with hdfs server for wrong file seperator
Posted by GitBox <gi...@apache.org>.
wangxianghu commented on pull request #2526:
URL: https://github.com/apache/hudi/pull/2526#issuecomment-773192292
@yanghua any other concern ?
----------------------------------------------------------------
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] MyLanPangzi commented on a change in pull request #2526: [HUDI-1420] HoodieTableMetaClient.getMarkerFolderPath works incorrectly on windows client with hdfs server for wrong file seperator
Posted by GitBox <gi...@apache.org>.
MyLanPangzi commented on a change in pull request #2526:
URL: https://github.com/apache/hudi/pull/2526#discussion_r569883592
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableMetaClient.java
##########
@@ -118,7 +120,7 @@ public HoodieTableMetaClient(Configuration conf, String basePath, boolean loadAc
ConsistencyGuardConfig consistencyGuardConfig, Option<TimelineLayoutVersion> layoutVersion,
String payloadClassName) {
LOG.info("Loading HoodieTableMetaClient from " + basePath);
- this.basePath = basePath;
+ this.basePath = FilenameUtils.separatorsToUnix(basePath);
Review comment:
If you do not add this line of code, the org.apache.hudi.common.table.TestHoodieTableMetaClient#checkMetadata test will not pass. because of org.apache.hudi.common.testutils.HoodieCommonTestHarness#tempDir using java.nio.file.Path, windows users cannot pass the test. but now the ci seems like failed.
----------------------------------------------------------------
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] wangxianghu merged pull request #2526: [HUDI-1420] HoodieTableMetaClient.getMarkerFolderPath works incorrectly on windows client with hdfs server for wrong file seperator
Posted by GitBox <gi...@apache.org>.
wangxianghu merged pull request #2526:
URL: https://github.com/apache/hudi/pull/2526
----------------------------------------------------------------
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] MyLanPangzi commented on a change in pull request #2526: [HUDI-1420] HoodieTableMetaClient.getMarkerFolderPath works incorrectly on windows client with hdfs server for wrong file seperator
Posted by GitBox <gi...@apache.org>.
MyLanPangzi commented on a change in pull request #2526:
URL: https://github.com/apache/hudi/pull/2526#discussion_r569972148
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableMetaClient.java
##########
@@ -118,7 +120,7 @@ public HoodieTableMetaClient(Configuration conf, String basePath, boolean loadAc
ConsistencyGuardConfig consistencyGuardConfig, Option<TimelineLayoutVersion> layoutVersion,
String payloadClassName) {
LOG.info("Loading HoodieTableMetaClient from " + basePath);
- this.basePath = basePath;
+ this.basePath = FilenameUtils.separatorsToUnix(basePath);
Review comment:
fixed and then ？
----------------------------------------------------------------
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] wangxianghu commented on a change in pull request #2526: [HUDI-1420] HoodieTableMetaClient.getMarkerFolderPath works incorrectly on windows client with hdfs server for wrong file seperator
Posted by GitBox <gi...@apache.org>.
wangxianghu commented on a change in pull request #2526:
URL: https://github.com/apache/hudi/pull/2526#discussion_r569922647
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableMetaClient.java
##########
@@ -118,7 +120,7 @@ public HoodieTableMetaClient(Configuration conf, String basePath, boolean loadAc
ConsistencyGuardConfig consistencyGuardConfig, Option<TimelineLayoutVersion> layoutVersion,
String payloadClassName) {
LOG.info("Loading HoodieTableMetaClient from " + basePath);
- this.basePath = basePath;
+ this.basePath = FilenameUtils.separatorsToUnix(basePath);
Review comment:
> If you do not add this line of code, the org.apache.hudi.common.table.TestHoodieTableMetaClient#checkMetadata test will not pass. because of org.apache.hudi.common.testutils.HoodieCommonTestHarness#tempDir using java.nio.file.Path, windows users cannot pass the test. but now the ci seems like failed.
ok， then
----------------------------------------------------------------
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] wangxianghu commented on a change in pull request #2526: [HUDI-1420] HoodieTableMetaClient.getMarkerFolderPath works incorrectly on windows client with hdfs server for wrong file seperator
Posted by GitBox <gi...@apache.org>.
wangxianghu commented on a change in pull request #2526:
URL: https://github.com/apache/hudi/pull/2526#discussion_r569505718
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableMetaClient.java
##########
@@ -118,7 +120,7 @@ public HoodieTableMetaClient(Configuration conf, String basePath, boolean loadAc
ConsistencyGuardConfig consistencyGuardConfig, Option<TimelineLayoutVersion> layoutVersion,
String payloadClassName) {
LOG.info("Loading HoodieTableMetaClient from " + basePath);
- this.basePath = basePath;
+ this.basePath = FilenameUtils.separatorsToUnix(basePath);
Review comment:
This change seems unnecessary， WDYT?
----------------------------------------------------------------
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] wangxianghu commented on a change in pull request #2526: [HUDI-1420] HoodieTableMetaClient.getMarkerFolderPath works incorrectly on windows client with hdfs server for wrong file seperator
Posted by GitBox <gi...@apache.org>.
wangxianghu commented on a change in pull request #2526:
URL: https://github.com/apache/hudi/pull/2526#discussion_r569506117
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableMetaClient.java
##########
@@ -45,6 +45,8 @@
import org.apache.log4j.LogManager;
import org.apache.log4j.Logger;
+import org.apache.commons.io.FilenameUtils;
+
import java.io.File;
Review comment:
`java.io.File` is not needed anymore, remove it
----------------------------------------------------------------
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] wangxianghu commented on a change in pull request #2526: [HUDI-1420] HoodieTableMetaClient.getMarkerFolderPath works incorrectly on windows client with hdfs server for wrong file seperator
Posted by GitBox <gi...@apache.org>.
wangxianghu commented on a change in pull request #2526:
URL: https://github.com/apache/hudi/pull/2526#discussion_r569959838
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableMetaClient.java
##########
@@ -118,7 +120,7 @@ public HoodieTableMetaClient(Configuration conf, String basePath, boolean loadAc
ConsistencyGuardConfig consistencyGuardConfig, Option<TimelineLayoutVersion> layoutVersion,
String payloadClassName) {
LOG.info("Loading HoodieTableMetaClient from " + basePath);
- this.basePath = basePath;
+ this.basePath = FilenameUtils.separatorsToUnix(basePath);
Review comment:
The ci failed, please fix it
```
[INFO] There are 2 errors reported by Checkstyle 8.18 with style/checkstyle.xml ruleset.
[ERROR] src/main/java/org/apache/hudi/common/table/HoodieTableMetaClient.java:[48,1] (imports) IllegalImport: Illegal import - org.apache.commons.io.FilenameUtils.
[ERROR] src/main/java/org/apache/hudi/common/table/HoodieTableMetaClient.java:[50,8] (imports) UnusedImports: Unused import - java.io.File.
```
----------------------------------------------------------------
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