You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Grant Henke (Code Review)" <ge...@cloudera.org> on 2018/06/01 15:22:30 UTC
[kudu-CR] [java] Move some methods to a SparkUtil class
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10572
Change subject: [java] Move some methods to a SparkUtil class
......................................................................
[java] Move some methods to a SparkUtil class
Moves some utility methods to a SparkUtil class
that is marked as private. These methods are useful
in the backup application work and adding them here
minimizes duplication.
Change-Id: Ib78ec895b25220668757432de287f6d0e5ed5ccc
---
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
A java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/SparkUtil.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala
4 files changed, 125 insertions(+), 80 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/10572/1
--
To view, visit http://gerrit.cloudera.org:8080/10572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib78ec895b25220668757432de287f6d0e5ed5ccc
Gerrit-Change-Number: 10572
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
[kudu-CR] [java] Move some methods to a SparkUtil class
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10572
to look at the new patch set (#2).
Change subject: [java] Move some methods to a SparkUtil class
......................................................................
[java] Move some methods to a SparkUtil class
Moves some utility methods to a SparkUtil class
that is marked as private. These methods are useful
in the backup application work and adding them here
minimizes duplication.
Change-Id: Ib78ec895b25220668757432de287f6d0e5ed5ccc
---
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
A java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/SparkUtil.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala
4 files changed, 125 insertions(+), 80 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/10572/2
--
To view, visit http://gerrit.cloudera.org:8080/10572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib78ec895b25220668757432de287f6d0e5ed5ccc
Gerrit-Change-Number: 10572
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [java] Move some methods to a SparkUtil class
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has removed a vote on this change.
Change subject: [java] Move some methods to a SparkUtil class
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/10572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ib78ec895b25220668757432de287f6d0e5ed5ccc
Gerrit-Change-Number: 10572
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
[kudu-CR] [java] Move some methods to a SparkUtil class
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/10572 )
Change subject: [java] Move some methods to a SparkUtil class
......................................................................
Patch Set 2:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/10572/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala
File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala:
http://gerrit.cloudera.org:8080/#/c/10572/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala@24
PS2, Line 24: import org.apache.kudu.ColumnSchema
> nit: import order.
Done
http://gerrit.cloudera.org:8080/#/c/10572/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala:
http://gerrit.cloudera.org:8080/#/c/10572/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala@187
PS2, Line 187: def createSchema(schema: StructType, keys: Seq[String]): Schema = {
> Why not remove this method if it is just calling kuduSchema?
I didn't want to break public API.
http://gerrit.cloudera.org:8080/#/c/10572/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/SparkUtil.scala
File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/SparkUtil.scala:
http://gerrit.cloudera.org:8080/#/c/10572/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/SparkUtil.scala@100
PS2, Line 100: private def createColumn(field: StructField, isKey: Boolean): ColumnSchema = {
> nit: method comment.
Done
http://gerrit.cloudera.org:8080/#/c/10572/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/SparkUtil.scala@102
PS2, Line 102: val col = new ColumnSchema.ColumnSchemaBuilder(field.name, kt).key(isKey).nullable(field.nullable)
> nit: exceeds line length limit.
Done
http://gerrit.cloudera.org:8080/#/c/10572/2/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala
File java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala:
http://gerrit.cloudera.org:8080/#/c/10572/2/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala@35
PS2, Line 35: import scala.collection.JavaConverters._
> Is this needed?
Done
--
To view, visit http://gerrit.cloudera.org:8080/10572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib78ec895b25220668757432de287f6d0e5ed5ccc
Gerrit-Change-Number: 10572
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Sun, 03 Jun 2018 21:05:01 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] Move some methods to a SparkUtil class
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10572 )
Change subject: [java] Move some methods to a SparkUtil class
......................................................................
[java] Move some methods to a SparkUtil class
Moves some utility methods to a SparkUtil class
that is marked as private. These methods are useful
in the backup application work and adding them here
minimizes duplication.
Change-Id: Ib78ec895b25220668757432de287f6d0e5ed5ccc
Reviewed-on: http://gerrit.cloudera.org:8080/10572
Reviewed-by: Hao Hao <ha...@cloudera.com>
Tested-by: Grant Henke <gr...@apache.org>
---
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
A java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/SparkUtil.scala
3 files changed, 131 insertions(+), 80 deletions(-)
Approvals:
Hao Hao: Looks good to me, approved
Grant Henke: Verified
--
To view, visit http://gerrit.cloudera.org:8080/10572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib78ec895b25220668757432de287f6d0e5ed5ccc
Gerrit-Change-Number: 10572
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
[kudu-CR] [java] Move some methods to a SparkUtil class
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Mike Percy, Kudu Jenkins, Adar Dembo, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10572
to look at the new patch set (#3).
Change subject: [java] Move some methods to a SparkUtil class
......................................................................
[java] Move some methods to a SparkUtil class
Moves some utility methods to a SparkUtil class
that is marked as private. These methods are useful
in the backup application work and adding them here
minimizes duplication.
Change-Id: Ib78ec895b25220668757432de287f6d0e5ed5ccc
---
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
A java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/SparkUtil.scala
3 files changed, 131 insertions(+), 80 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/10572/3
--
To view, visit http://gerrit.cloudera.org:8080/10572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib78ec895b25220668757432de287f6d0e5ed5ccc
Gerrit-Change-Number: 10572
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
[kudu-CR] [java] Move some methods to a SparkUtil class
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/10572 )
Change subject: [java] Move some methods to a SparkUtil class
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/10572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib78ec895b25220668757432de287f6d0e5ed5ccc
Gerrit-Change-Number: 10572
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Tue, 05 Jun 2018 22:22:13 +0000
Gerrit-HasComments: No
[kudu-CR] [java] Move some methods to a SparkUtil class
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/10572 )
Change subject: [java] Move some methods to a SparkUtil class
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/10572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib78ec895b25220668757432de287f6d0e5ed5ccc
Gerrit-Change-Number: 10572
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Wed, 06 Jun 2018 15:23:58 +0000
Gerrit-HasComments: No
[kudu-CR] [java] Move some methods to a SparkUtil class
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/10572 )
Change subject: [java] Move some methods to a SparkUtil class
......................................................................
Patch Set 2:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/10572/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala
File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala:
http://gerrit.cloudera.org:8080/#/c/10572/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala@24
PS2, Line 24: import org.apache.kudu.ColumnSchema
nit: import order.
http://gerrit.cloudera.org:8080/#/c/10572/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala:
http://gerrit.cloudera.org:8080/#/c/10572/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala@187
PS2, Line 187: def createSchema(schema: StructType, keys: Seq[String]): Schema = {
Why not remove this method if it is just calling kuduSchema?
http://gerrit.cloudera.org:8080/#/c/10572/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala@192
PS2, Line 192: def kuduType(dt: DataType) : Type = {
Same here.
http://gerrit.cloudera.org:8080/#/c/10572/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/SparkUtil.scala
File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/SparkUtil.scala:
http://gerrit.cloudera.org:8080/#/c/10572/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/SparkUtil.scala@40
PS2, Line 40: * @param dt the Spark SQL type
nit: add an empty line.
http://gerrit.cloudera.org:8080/#/c/10572/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/SparkUtil.scala@100
PS2, Line 100: private def createColumn(field: StructField, isKey: Boolean): ColumnSchema = {
nit: method comment.
http://gerrit.cloudera.org:8080/#/c/10572/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/SparkUtil.scala@102
PS2, Line 102: val col = new ColumnSchema.ColumnSchemaBuilder(field.name, kt).key(isKey).nullable(field.nullable)
nit: exceeds line length limit.
http://gerrit.cloudera.org:8080/#/c/10572/2/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala
File java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala:
http://gerrit.cloudera.org:8080/#/c/10572/2/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala@35
PS2, Line 35: import scala.collection.JavaConverters._
Is this needed?
--
To view, visit http://gerrit.cloudera.org:8080/10572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib78ec895b25220668757432de287f6d0e5ed5ccc
Gerrit-Change-Number: 10572
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Fri, 01 Jun 2018 19:02:01 +0000
Gerrit-HasComments: Yes