Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support read hadoop conf file #105

Merged
merged 6 commits into from
Nov 26, 2019
Merged

Support read hadoop conf file #105

merged 6 commits into from
Nov 26, 2019

Conversation

Alwayswaiting
Copy link
Contributor

implement #104

@CLAassistant
Copy link

CLAassistant commented Nov 9, 2019 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Nov 9, 2019 •
edited
Loading

Codecov Report

Merging #105 into master will increase coverage by 0.45%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #105      +/-   ##
============================================
+ Coverage     74.93%   75.39%   +0.45%     
- Complexity      609      611       +2     
============================================
  Files            63       63              
  Lines          2346     2337       -9     
  Branches        246      245       -1     
============================================
+ Hits           1758     1762       +4     
+ Misses          436      425      -11     
+ Partials        152      150       -2
Impacted Files Coverage Δ Complexity Δ
...u/hugegraph/loader/reader/hdfs/HDFSFileReader.java 78.68% <100%> (+14.3%) 12 <0> (ø) ⬇️
...baidu/hugegraph/loader/source/hdfs/HDFSSource.java 100% <100%> (ø) 6 <3> (+2) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 63aea9c...884ac76. Read the comment docs.

@Alwayswaiting Alwayswaiting force-pushed the hadoop-conf-file branch 2 times, most recently from 6edc835 to 126286b Compare November 13, 2019 12:36
@Alwayswaiting Alwayswaiting force-pushed the hadoop-conf-file branch 3 times, most recently from 0f8bc32 to 27ebd92 Compare November 14, 2019 07:10
javeme
javeme reviewed Nov 14, 2019
View reviewed changes
src/main/java/com/baidu/hugegraph/loader/source/hdfs/HDFSSource.java Outdated Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved
@Alwayswaiting Alwayswaiting force-pushed the hadoop-conf-file branch 5 times, most recently from 811dcff to 759e202 Compare November 18, 2019 02:25
zhoney
zhoney requested changes Nov 18, 2019
View reviewed changes
src/test/java/com/baidu/hugegraph/loader/test/functional/FileLoadTest.java Outdated Show resolved Hide resolved
src/test/java/com/baidu/hugegraph/loader/test/functional/FileLoadTest.java Outdated Show resolved Hide resolved
src/test/java/com/baidu/hugegraph/loader/test/functional/FileLoadTest.java
@AfterClass
public static void tearDown() {
ioUtil.close();
public String structPath(String fileName) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems same with static configPath(String)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let it call configPath(String), and it will be override by HDFSLoadTest

src/test/java/com/baidu/hugegraph/loader/test/functional/FileLoadTest.java Outdated
@@ -305,6 +279,8 @@ public void testVertexIdExceedLimit() {
};
Assert.assertThrows(ParseException.class, () -> {
HugeGraphLoader.main(args);
}, e -> {
System.out.println(e);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delete

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add Assert.assertTrue(e.contains("xx")) if necessary

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing special, delete it.

javeme
javeme requested changes Nov 18, 2019
View reviewed changes
src/test/java/com/baidu/hugegraph/loader/test/functional/FileLoadTest.java Outdated
@@ -305,6 +279,8 @@ public void testVertexIdExceedLimit() {
};
Assert.assertThrows(ParseException.class, () -> {
HugeGraphLoader.main(args);
}, e -> {
System.out.println(e);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add Assert.assertTrue(e.contains("xx")) if necessary

src/main/java/com/baidu/hugegraph/loader/reader/hdfs/HDFSFileReader.java Outdated Show resolved Hide resolved
src/test/resources/hdfs_with_invalid_core_site_path/core-site.xml Show resolved Hide resolved
src/test/java/com/baidu/hugegraph/loader/test/functional/HDFSLoadTest.java Outdated Show resolved Hide resolved
Linary
Linary previously approved these changes Nov 25, 2019
View reviewed changes
javeme
javeme previously approved these changes Nov 25, 2019
View reviewed changes
zhoney
zhoney requested changes Nov 25, 2019
View reviewed changes
src/main/java/com/baidu/hugegraph/loader/reader/hdfs/HDFSFileReader.java Outdated
return conf;
}

private static void checkExist(FileSystem fs, Path path) {
LoadException e = null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems useless changes, why?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeap

src/test/java/com/baidu/hugegraph/loader/test/functional/FileLoadTest.java Outdated Show resolved Hide resolved
src/test/java/com/baidu/hugegraph/loader/test/functional/FileLoadTest.java Outdated Show resolved Hide resolved
src/test/java/com/baidu/hugegraph/loader/test/functional/FileLoadTest.java Outdated
"value_list_property_in_csv_file_with_same_delimiter/struct.json"),
"-s", configPath(
"-s", structPath(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto. Please handle all same conditions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

total 4 locations

javeme
javeme previously approved these changes Nov 26, 2019
View reviewed changes
@zhoney zhoney merged commit d8cf21f into apache:master Nov 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Linary Linary Linary approved these changes

@javeme javeme javeme approved these changes

@zhoney zhoney zhoney approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Alwayswaiting @CLAassistant @Linary @javeme @zhoney