You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2019/01/10 03:47:39 UTC
[kudu-CR] [examples] a small update on SparkExample
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12208
Change subject: [examples] a small update on SparkExample
......................................................................
[examples] a small update on SparkExample
Updated the Spark example:
* added replication factor as a parameter for the application
(by default set to 1)
* don't try to drop the test table if it hasn't been created
* catch and log the information about caught exceptions
* other petty changes
Change-Id: Icf8c4e675ca6c240582242658fa8173a1ccd271d
---
M examples/scala/spark-example/src/main/scala/org/apache/kudu/spark/examples/SparkExample.scala
1 file changed, 35 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/08/12208/1
--
To view, visit http://gerrit.cloudera.org:8080/12208
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Icf8c4e675ca6c240582242658fa8173a1ccd271d
Gerrit-Change-Number: 12208
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
[kudu-CR] [examples] a small update on SparkExample
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/12208 )
Change subject: [examples] a small update on SparkExample
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/12208
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf8c4e675ca6c240582242658fa8173a1ccd271d
Gerrit-Change-Number: 12208
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mitch Barnett <mb...@cloudera.com>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 11 Jan 2019 21:10:21 +0000
Gerrit-HasComments: No
[kudu-CR] [examples] a small update on SparkExample
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12208 )
Change subject: [examples] a small update on SparkExample
......................................................................
Patch Set 2:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/12208/2/examples/scala/spark-example/src/main/scala/org/apache/kudu/spark/examples/SparkExample.scala
File examples/scala/spark-example/src/main/scala/org/apache/kudu/spark/examples/SparkExample.scala:
http://gerrit.cloudera.org:8080/#/c/12208/2/examples/scala/spark-example/src/main/scala/org/apache/kudu/spark/examples/SparkExample.scala@43
PS2, Line 43: avaialble
> typo
Done
http://gerrit.cloudera.org:8080/#/c/12208/2/examples/scala/spark-example/src/main/scala/org/apache/kudu/spark/examples/SparkExample.scala@44
PS2, Line 44: val tableReplicasNum: Int = Integer.getInteger("tableReplicasNum", 1)
> Since this is an example that a user with no experience may use, can you ad
Done
http://gerrit.cloudera.org:8080/#/c/12208/2/examples/scala/spark-example/src/main/scala/org/apache/kudu/spark/examples/SparkExample.scala@44
PS2, Line 44: tableReplicasNum: Int = Integer.getInteger("tableReplicasNum"
> nit: maybe tableNumReplicas?
Done
http://gerrit.cloudera.org:8080/#/c/12208/2/examples/scala/spark-example/src/main/scala/org/apache/kudu/spark/examples/SparkExample.scala@81
PS2, Line 81: if (!kuduContext.tableExists(tableName)) {
> Instead should we just fail if it already exists?
That's a good idea since the test doesn't do introspection of the existing table and requires some schema. Andrew and I talked about that as well, but for the first cut decided to go with the original logic.
OK, let me update that.
http://gerrit.cloudera.org:8080/#/c/12208/2/examples/scala/spark-example/src/main/scala/org/apache/kudu/spark/examples/SparkExample.scala@81
PS2, Line 81: if (!kuduContext.tableExists(tableName)) {
> At first I leaned towards going ahead if the table exists, but on second th
Indeed.
--
To view, visit http://gerrit.cloudera.org:8080/12208
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf8c4e675ca6c240582242658fa8173a1ccd271d
Gerrit-Change-Number: 12208
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mitch Barnett <mb...@cloudera.com>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 10 Jan 2019 06:33:18 +0000
Gerrit-HasComments: Yes
[kudu-CR] [examples] a small update on SparkExample
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/12208 )
Change subject: [examples] a small update on SparkExample
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/12208/2/examples/scala/spark-example/src/main/scala/org/apache/kudu/spark/examples/SparkExample.scala
File examples/scala/spark-example/src/main/scala/org/apache/kudu/spark/examples/SparkExample.scala:
http://gerrit.cloudera.org:8080/#/c/12208/2/examples/scala/spark-example/src/main/scala/org/apache/kudu/spark/examples/SparkExample.scala@44
PS2, Line 44: val tableReplicasNum: Int = Integer.getInteger("tableReplicasNum", 1)
Since this is an example that a user with no experience may use, can you add a comment about a replication factor of 1 being a bad idea. I understand it's set here for maximum environment compatibility.
http://gerrit.cloudera.org:8080/#/c/12208/2/examples/scala/spark-example/src/main/scala/org/apache/kudu/spark/examples/SparkExample.scala@81
PS2, Line 81: if (!kuduContext.tableExists(tableName)) {
Instead should we just fail if it already exists?
--
To view, visit http://gerrit.cloudera.org:8080/12208
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf8c4e675ca6c240582242658fa8173a1ccd271d
Gerrit-Change-Number: 12208
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 10 Jan 2019 03:55:56 +0000
Gerrit-HasComments: Yes
[kudu-CR] [examples] a small update on SparkExample
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12208 )
Change subject: [examples] a small update on SparkExample
......................................................................
Patch Set 3: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12208/3/examples/scala/spark-example/src/main/scala/org/apache/kudu/spark/examples/SparkExample.scala
File examples/scala/spark-example/src/main/scala/org/apache/kudu/spark/examples/SparkExample.scala:
http://gerrit.cloudera.org:8080/#/c/12208/3/examples/scala/spark-example/src/main/scala/org/apache/kudu/spark/examples/SparkExample.scala@130
PS3, Line 130: // Catch, log and re-throw. Not the best practice, but this is a very
: // simplistic example.
: case unknown : Throwable => logger.error(s"got an exception: " + unknown)
: throw unknown
nit: Will this display the exception twice? If so, maybe just throw? Or just log?
--
To view, visit http://gerrit.cloudera.org:8080/12208
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf8c4e675ca6c240582242658fa8173a1ccd271d
Gerrit-Change-Number: 12208
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mitch Barnett <mb...@cloudera.com>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 11 Jan 2019 01:20:56 +0000
Gerrit-HasComments: Yes
[kudu-CR] [examples] a small update on SparkExample
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12208 )
Change subject: [examples] a small update on SparkExample
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12208/3/examples/scala/spark-example/src/main/scala/org/apache/kudu/spark/examples/SparkExample.scala
File examples/scala/spark-example/src/main/scala/org/apache/kudu/spark/examples/SparkExample.scala:
http://gerrit.cloudera.org:8080/#/c/12208/3/examples/scala/spark-example/src/main/scala/org/apache/kudu/spark/examples/SparkExample.scala@130
PS3, Line 130: // Catch, log and re-throw. Not the best practice, but this is a very
: // simplistic example.
: case unknown : Throwable => logger.error(s"got an exception: " + unknown)
: throw unknown
> nit: Will this display the exception twice? If so, maybe just throw? Or jus
Yep, I guess so. Anyway, I think I'll address this separately once I get a chance -- it seems I'll be touching this small example once more soon.
--
To view, visit http://gerrit.cloudera.org:8080/12208
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf8c4e675ca6c240582242658fa8173a1ccd271d
Gerrit-Change-Number: 12208
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mitch Barnett <mb...@cloudera.com>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Sat, 19 Jan 2019 00:28:04 +0000
Gerrit-HasComments: Yes
[kudu-CR] [examples] a small update on SparkExample
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12208 )
Change subject: [examples] a small update on SparkExample
......................................................................
[examples] a small update on SparkExample
Updated the Spark example:
* added replication factor as a parameter for the application
(by default set to 1)
* don't try to drop the test table if it hasn't been created
* catch and log the information about caught exceptions
* other petty changes
Change-Id: Icf8c4e675ca6c240582242658fa8173a1ccd271d
Reviewed-on: http://gerrit.cloudera.org:8080/12208
Tested-by: Kudu Jenkins
Reviewed-by: Andrew Wong <aw...@cloudera.com>
Reviewed-by: Will Berkeley <wd...@gmail.com>
---
M examples/scala/spark-example/src/main/scala/org/apache/kudu/spark/examples/SparkExample.scala
1 file changed, 54 insertions(+), 17 deletions(-)
Approvals:
Kudu Jenkins: Verified
Andrew Wong: Looks good to me, but someone else must approve
Will Berkeley: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/12208
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Icf8c4e675ca6c240582242658fa8173a1ccd271d
Gerrit-Change-Number: 12208
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mitch Barnett <mb...@cloudera.com>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [examples] a small update on SparkExample
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12208 )
Change subject: [examples] a small update on SparkExample
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/12208/2/examples/scala/spark-example/src/main/scala/org/apache/kudu/spark/examples/SparkExample.scala
File examples/scala/spark-example/src/main/scala/org/apache/kudu/spark/examples/SparkExample.scala:
http://gerrit.cloudera.org:8080/#/c/12208/2/examples/scala/spark-example/src/main/scala/org/apache/kudu/spark/examples/SparkExample.scala@43
PS2, Line 43: avaialble
typo
http://gerrit.cloudera.org:8080/#/c/12208/2/examples/scala/spark-example/src/main/scala/org/apache/kudu/spark/examples/SparkExample.scala@44
PS2, Line 44: tableReplicasNum: Int = Integer.getInteger("tableReplicasNum"
nit: maybe tableNumReplicas?
http://gerrit.cloudera.org:8080/#/c/12208/2/examples/scala/spark-example/src/main/scala/org/apache/kudu/spark/examples/SparkExample.scala@81
PS2, Line 81: if (!kuduContext.tableExists(tableName)) {
> Instead should we just fail if it already exists?
At first I leaned towards going ahead if the table exists, but on second thought maybe we should fail since we expect a specific schema, etc.
--
To view, visit http://gerrit.cloudera.org:8080/12208
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf8c4e675ca6c240582242658fa8173a1ccd271d
Gerrit-Change-Number: 12208
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mitch Barnett <mb...@cloudera.com>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 10 Jan 2019 04:01:36 +0000
Gerrit-HasComments: Yes
[kudu-CR] [examples] a small update on SparkExample
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Kudu Jenkins, Andrew Wong, Mitch Barnett, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12208
to look at the new patch set (#3).
Change subject: [examples] a small update on SparkExample
......................................................................
[examples] a small update on SparkExample
Updated the Spark example:
* added replication factor as a parameter for the application
(by default set to 1)
* don't try to drop the test table if it hasn't been created
* catch and log the information about caught exceptions
* other petty changes
Change-Id: Icf8c4e675ca6c240582242658fa8173a1ccd271d
---
M examples/scala/spark-example/src/main/scala/org/apache/kudu/spark/examples/SparkExample.scala
1 file changed, 54 insertions(+), 17 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/08/12208/3
--
To view, visit http://gerrit.cloudera.org:8080/12208
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icf8c4e675ca6c240582242658fa8173a1ccd271d
Gerrit-Change-Number: 12208
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mitch Barnett <mb...@cloudera.com>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [examples] a small update on SparkExample
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12208
to look at the new patch set (#2).
Change subject: [examples] a small update on SparkExample
......................................................................
[examples] a small update on SparkExample
Updated the Spark example:
* added replication factor as a parameter for the application
(by default set to 1)
* don't try to drop the test table if it hasn't been created
* catch and log the information about caught exceptions
* other petty changes
Change-Id: Icf8c4e675ca6c240582242658fa8173a1ccd271d
---
M examples/scala/spark-example/src/main/scala/org/apache/kudu/spark/examples/SparkExample.scala
1 file changed, 35 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/08/12208/2
--
To view, visit http://gerrit.cloudera.org:8080/12208
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icf8c4e675ca6c240582242658fa8173a1ccd271d
Gerrit-Change-Number: 12208
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)