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/01/01 00:08:49 UTC
[GitHub] [incubator-hudi] lamber-ken opened a new pull request #1167:
[HUDI-484] Fix NPE when reading IncrementalPull.sqltemplate in
HiveIncrementalPuller
lamber-ken opened a new pull request #1167: [HUDI-484] Fix NPE when reading IncrementalPull.sqltemplate in HiveIncrementalPuller
URL: https://github.com/apache/incubator-hudi/pull/1167
## What is the purpose of the pull request
When using `class.getResourceAsStream` method, the resource name should begins with a '/'.
More detail, please visit https://lists.apache.org/thread.html/9c7ab9b8cf63d8f1f7fd72a0aba870976e81d9c50ed53db80f082284%40%3Cdev.hudi.apache.org%3E
## Brief change log
- Change "IncrementalPull.sqltemplate" to "/IncrementalPull.sqltemplate".
## Verify this pull request
This pull request is code cleanup without any test coverage.
## 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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] vinothchandar merged pull request #1167:
[HUDI-484] Fix NPE when reading IncrementalPull.sqltemplate in
HiveIncrementalPuller
Posted by GitBox <gi...@apache.org>.
vinothchandar merged pull request #1167: [HUDI-484] Fix NPE when reading IncrementalPull.sqltemplate in HiveIncrementalPuller
URL: https://github.com/apache/incubator-hudi/pull/1167
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] vinothchandar commented on a change in pull
request #1167: [HUDI-484] Fix NPE when reading IncrementalPull.sqltemplate
in HiveIncrementalPuller
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on a change in pull request #1167: [HUDI-484] Fix NPE when reading IncrementalPull.sqltemplate in HiveIncrementalPuller
URL: https://github.com/apache/incubator-hudi/pull/1167#discussion_r363073378
##########
File path: hudi-utilities/src/test/java/org/apache/hudi/utilities/TestHiveIncrementalPuller.java
##########
@@ -0,0 +1,27 @@
+package org.apache.hudi.utilities;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+public class TestHiveIncrementalPuller {
+
+ private HiveIncrementalPuller.Config config;
+
+ @Before
+ public void setup() {
+ config = new HiveIncrementalPuller.Config();
+ }
+
+ @Test
+ public void testInitHiveIncrementalPuller() throws Exception {
Review comment:
nit: do we still need the `throws Exception`? :)
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] lamber-ken commented on a change in pull request
#1167: [HUDI-484] Fix NPE when reading IncrementalPull.sqltemplate in
HiveIncrementalPuller
Posted by GitBox <gi...@apache.org>.
lamber-ken commented on a change in pull request #1167: [HUDI-484] Fix NPE when reading IncrementalPull.sqltemplate in HiveIncrementalPuller
URL: https://github.com/apache/incubator-hudi/pull/1167#discussion_r363073651
##########
File path: hudi-utilities/src/test/java/org/apache/hudi/utilities/TestHiveIncrementalPuller.java
##########
@@ -0,0 +1,27 @@
+package org.apache.hudi.utilities;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+public class TestHiveIncrementalPuller {
+
+ private HiveIncrementalPuller.Config config;
+
+ @Before
+ public void setup() {
+ config = new HiveIncrementalPuller.Config();
+ }
+
+ @Test
+ public void testInitHiveIncrementalPuller() throws Exception {
Review comment:
Good catch 👍 . Sorry, I forgot to delete 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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] lamber-ken commented on a change in pull request
#1167: [HUDI-484] Fix NPE when reading IncrementalPull.sqltemplate in
HiveIncrementalPuller
Posted by GitBox <gi...@apache.org>.
lamber-ken commented on a change in pull request #1167: [HUDI-484] Fix NPE when reading IncrementalPull.sqltemplate in HiveIncrementalPuller
URL: https://github.com/apache/incubator-hudi/pull/1167#discussion_r363054144
##########
File path: hudi-utilities/src/test/java/org/apache/hudi/utilities/TestHiveIncrementalPuller.java
##########
@@ -0,0 +1,24 @@
+package org.apache.hudi.utilities;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+public class TestHiveIncrementalPuller {
+
+ private HiveIncrementalPuller.Config config;
+
+ @Before
+ public void init() {
+ config = new HiveIncrementalPuller.Config();
+ }
+
+ @Test
+ public void testInitHiveIncrementalPuller() throws Exception {
+
+ HiveIncrementalPuller puller = new HiveIncrementalPuller(config);
Review comment:
Done.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] lamber-ken commented on issue #1167: [HUDI-484]
Fix NPE when reading IncrementalPull.sqltemplate in HiveIncrementalPuller
Posted by GitBox <gi...@apache.org>.
lamber-ken commented on issue #1167: [HUDI-484] Fix NPE when reading IncrementalPull.sqltemplate in HiveIncrementalPuller
URL: https://github.com/apache/incubator-hudi/pull/1167#issuecomment-570071309
> Could we add a unit test for this tool? its historically been not very popular.. But better to cover this for the future
Done. 😄
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] lamber-ken commented on issue #1167: [HUDI-484]
Fix NPE when reading IncrementalPull.sqltemplate in HiveIncrementalPuller
Posted by GitBox <gi...@apache.org>.
lamber-ken commented on issue #1167: [HUDI-484] Fix NPE when reading IncrementalPull.sqltemplate in HiveIncrementalPuller
URL: https://github.com/apache/incubator-hudi/pull/1167#issuecomment-570628522
> Sure we can file a new JIRA for end-end test of HiveIncrementalPuller .. We can merge this once you address the test case comment. Thanks @lamber-ken !
You are welcome, I'll ping you later when finishing. I am debuging other issues(need switch branch).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] lamber-ken commented on issue #1167: [HUDI-484]
Fix NPE when reading IncrementalPull.sqltemplate in HiveIncrementalPuller
Posted by GitBox <gi...@apache.org>.
lamber-ken commented on issue #1167: [HUDI-484] Fix NPE when reading IncrementalPull.sqltemplate in HiveIncrementalPuller
URL: https://github.com/apache/incubator-hudi/pull/1167#issuecomment-570117281
> I know this probably just tests the NPE.. but little more love and add a actual valid working test case that does a real pull? :) , in the spirit of improving the tool?
Reasonable, can split it to a new thread that end to end test the HiveIncrementalPuller? because this pr just fix the NPE.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] vinothchandar commented on a change in pull
request #1167: [HUDI-484] Fix NPE when reading IncrementalPull.sqltemplate
in HiveIncrementalPuller
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on a change in pull request #1167: [HUDI-484] Fix NPE when reading IncrementalPull.sqltemplate in HiveIncrementalPuller
URL: https://github.com/apache/incubator-hudi/pull/1167#discussion_r362366023
##########
File path: hudi-utilities/src/test/java/org/apache/hudi/utilities/TestHiveIncrementalPuller.java
##########
@@ -0,0 +1,24 @@
+package org.apache.hudi.utilities;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+public class TestHiveIncrementalPuller {
+
+ private HiveIncrementalPuller.Config config;
+
+ @Before
+ public void init() {
+ config = new HiveIncrementalPuller.Config();
+ }
+
+ @Test
+ public void testInitHiveIncrementalPuller() throws Exception {
+
+ HiveIncrementalPuller puller = new HiveIncrementalPuller(config);
Review comment:
Not following this.. a constructor can either throw an exception or return a non-null object right? should we be using a try-catch and calling `fail` from the catch block to ensure the NPE is not thrown?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] lamber-ken commented on a change in pull request
#1167: [HUDI-484] Fix NPE when reading IncrementalPull.sqltemplate in
HiveIncrementalPuller
Posted by GitBox <gi...@apache.org>.
lamber-ken commented on a change in pull request #1167: [HUDI-484] Fix NPE when reading IncrementalPull.sqltemplate in HiveIncrementalPuller
URL: https://github.com/apache/incubator-hudi/pull/1167#discussion_r363054122
##########
File path: hudi-utilities/src/test/java/org/apache/hudi/utilities/TestHiveIncrementalPuller.java
##########
@@ -0,0 +1,24 @@
+package org.apache.hudi.utilities;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+public class TestHiveIncrementalPuller {
+
+ private HiveIncrementalPuller.Config config;
+
+ @Before
+ public void init() {
Review comment:
Done.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] vinothchandar commented on issue #1167: [HUDI-484]
Fix NPE when reading IncrementalPull.sqltemplate in HiveIncrementalPuller
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on issue #1167: [HUDI-484] Fix NPE when reading IncrementalPull.sqltemplate in HiveIncrementalPuller
URL: https://github.com/apache/incubator-hudi/pull/1167#issuecomment-570012389
Could we add a unit test for this tool? its historically been not very popular.. But better to cover this for the future
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] lamber-ken commented on issue #1167: [HUDI-484]
Fix NPE when reading IncrementalPull.sqltemplate in HiveIncrementalPuller
Posted by GitBox <gi...@apache.org>.
lamber-ken commented on issue #1167: [HUDI-484] Fix NPE when reading IncrementalPull.sqltemplate in HiveIncrementalPuller
URL: https://github.com/apache/incubator-hudi/pull/1167#issuecomment-570819065
hi @vinothchandar, thank you for your guidance, all review comments are addressed and fixed.
And filed an issue about end to end to check HiveIncrementalPuller. https://issues.apache.org/jira/browse/HUDI-498
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] vinothchandar commented on issue #1167: [HUDI-484]
Fix NPE when reading IncrementalPull.sqltemplate in HiveIncrementalPuller
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on issue #1167: [HUDI-484] Fix NPE when reading IncrementalPull.sqltemplate in HiveIncrementalPuller
URL: https://github.com/apache/incubator-hudi/pull/1167#issuecomment-570273645
Sure we can file a new JIRA for end-end test of HiveIncrementalPuller .. We can merge this once you address the test case comment. Thanks @lamber-ken !
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] lamber-ken commented on a change in pull request
#1167: [HUDI-484] Fix NPE when reading IncrementalPull.sqltemplate in
HiveIncrementalPuller
Posted by GitBox <gi...@apache.org>.
lamber-ken commented on a change in pull request #1167: [HUDI-484] Fix NPE when reading IncrementalPull.sqltemplate in HiveIncrementalPuller
URL: https://github.com/apache/incubator-hudi/pull/1167#discussion_r362369737
##########
File path: hudi-utilities/src/test/java/org/apache/hudi/utilities/TestHiveIncrementalPuller.java
##########
@@ -0,0 +1,24 @@
+package org.apache.hudi.utilities;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+public class TestHiveIncrementalPuller {
+
+ private HiveIncrementalPuller.Config config;
+
+ @Before
+ public void init() {
+ config = new HiveIncrementalPuller.Config();
+ }
+
+ @Test
+ public void testInitHiveIncrementalPuller() throws Exception {
+
+ HiveIncrementalPuller puller = new HiveIncrementalPuller(config);
Review comment:
Got it, thank you for your guidance.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-hudi] vinothchandar commented on a change in pull
request #1167: [HUDI-484] Fix NPE when reading IncrementalPull.sqltemplate
in HiveIncrementalPuller
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on a change in pull request #1167: [HUDI-484] Fix NPE when reading IncrementalPull.sqltemplate in HiveIncrementalPuller
URL: https://github.com/apache/incubator-hudi/pull/1167#discussion_r362365869
##########
File path: hudi-utilities/src/test/java/org/apache/hudi/utilities/TestHiveIncrementalPuller.java
##########
@@ -0,0 +1,24 @@
+package org.apache.hudi.utilities;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+public class TestHiveIncrementalPuller {
+
+ private HiveIncrementalPuller.Config config;
+
+ @Before
+ public void init() {
Review comment:
rename to `setup()`? most of our tests are that way I 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services