You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@kudu.apache.org by "Will Berkeley (Code Review)" <ge...@cloudera.org> on 2016/09/06 19:12:08 UTC

[kudu-CR] API and style improvements to the Kudu Flume Sink

Will Berkeley has uploaded a new change for review.

http://gerrit.cloudera.org:8080/4320

Change subject: API and style improvements to the Kudu Flume Sink
......................................................................

API and style improvements to the Kudu Flume Sink

This patch cleans up the Flume integration code a bit. Specifically:
1. s/EventProducer/OperationsProducer/ since the "KuduEventProducer"
consumed Flume Events and produced Kudu Operations.
2. The KuduOperationsProducer API was changed. Now the initialize
method takes just a KuduTable, and should be called once to initialize
the KuduOperationsProducer after it is configured. The getOperations
method now takes the Event instead.
3. The close method for a KuduOperationsProducer is now called when
the KuduSink is called. Previously this was implied so in a comment
but was not true.
4. General clean-up and style improvements.

Change-Id: I357df8cac7daa6ce105f9568cc3af09697032eb6
---
R java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducer.java
D java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduEventProducer.java
A java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduOperationsProducer.java
M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSink.java
M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSinkConfigurationConstants.java
D java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/SimpleKeyedKuduEventProducer.java
A java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/SimpleKeyedKuduOperationsProducer.java
R java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/SimpleKuduOperationsProducer.java
R java/kudu-flume-sink/src/test/avro/testAvroKuduOperationsProducer.avsc
R java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducerTest.java
R java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KeyedKuduOperationsProducerTest.java
11 files changed, 318 insertions(+), 288 deletions(-)

git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/4320/1
--
To view, visit http://gerrit.cloudera.org:8080/4320
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I357df8cac7daa6ce105f9568cc3af09697032eb6
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>

[kudu-CR] API and style improvements to the Kudu Flume Sink

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.

Mike Percy has posted comments on this change.

Change subject: API and style improvements to the Kudu Flume Sink
......................................................................


Patch Set 1:

I also added Ara to the review in case he has input on this

-- 
To view, visit http://gerrit.cloudera.org:8080/4320
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I357df8cac7daa6ce105f9568cc3af09697032eb6
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Ara Ebrahimi <ar...@argyledata.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No

[kudu-CR] API and style improvements to the Kudu Flume Sink

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.

Mike Percy has posted comments on this change.

Change subject: API and style improvements to the Kudu Flume Sink
......................................................................


Patch Set 2:

> Do we also want something in the release notes about the API change
 > + AvroKuduOperationsProducer?

Looks good. Yes, please add to the release notes for 1.0.0 in the Incompatibility section as well as new features.

I think the AvroKuduOperationsProducer would also make a good topic for a blog post, if / when you have time, because it makes integrating with Kudu so easy.

-- 
To view, visit http://gerrit.cloudera.org:8080/4320
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I357df8cac7daa6ce105f9568cc3af09697032eb6
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Ara Ebrahimi <ar...@argyledata.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: No

[kudu-CR] API and style improvements to the Kudu Flume Sink

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.

Kudu Jenkins has posted comments on this change.

Change subject: API and style improvements to the Kudu Flume Sink
......................................................................


Patch Set 3:

Build Started http://104.196.14.100/job/kudu-gerrit/3284/

-- 
To view, visit http://gerrit.cloudera.org:8080/4320
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I357df8cac7daa6ce105f9568cc3af09697032eb6
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Ara Ebrahimi <ar...@argyledata.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: No

[kudu-CR] API and style improvements to the Kudu Flume Sink

Posted by "Ara Ebrahimi (Code Review)" <ge...@cloudera.org>.

Ara Ebrahimi has posted comments on this change.

Change subject: API and style improvements to the Kudu Flume Sink
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4320/1/java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSink.java
File java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSink.java:

Line 101:   private KuduOperationsProducer eventProducer;
Shouldn't this also be called operationsProducer instead of eventProducer?


-- 
To view, visit http://gerrit.cloudera.org:8080/4320
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I357df8cac7daa6ce105f9568cc3af09697032eb6
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Ara Ebrahimi <ar...@argyledata.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] API and style improvements to the Kudu Flume Sink

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.

Kudu Jenkins has posted comments on this change.

Change subject: API and style improvements to the Kudu Flume Sink
......................................................................


Patch Set 1:

Build Started http://104.196.14.100/job/kudu-gerrit/3237/

-- 
To view, visit http://gerrit.cloudera.org:8080/4320
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I357df8cac7daa6ce105f9568cc3af09697032eb6
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] API and style improvements to the Kudu Flume Sink

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.

Mike Percy has posted comments on this change.

Change subject: API and style improvements to the Kudu Flume Sink
......................................................................


Patch Set 3: Code-Review+2

> don't forget about RegexpKuduEventProducer,
 > which I need to rebase and refactor as RegexpKuduOperationsProducer.
 > Will try to do that later today.

That doesn't block this patch, does it? This can be committed and you can rebase it onto master.

-- 
To view, visit http://gerrit.cloudera.org:8080/4320
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I357df8cac7daa6ce105f9568cc3af09697032eb6
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Ara Ebrahimi <ar...@argyledata.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: No

[kudu-CR] API and style improvements to the Kudu Flume Sink

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.

Mike Percy has posted comments on this change.

Change subject: API and style improvements to the Kudu Flume Sink
......................................................................


Patch Set 1:

(7 comments)

Looks like good changes, and if we are going to break the API then we should do it before Kudu 1.0

http://gerrit.cloudera.org:8080/#/c/4320/1/java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducer.java
File java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducer.java:

Line 243:               String.format("Failed to coerce value for column `%s` to type %s",
nit: if we are going to quote, let's use single-quotes, not backquotes here. backquotes make me think of commands executed in bash


http://gerrit.cloudera.org:8080/#/c/4320/1/java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduOperationsProducer.java
File java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduOperationsProducer.java:

Line 36: public interface KuduOperationsProducer extends Configurable {
How about also: extends AutoCloseable


http://gerrit.cloudera.org:8080/#/c/4320/1/java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSink.java
File java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSink.java:

Line 145:     eventProducer.close();
I think we should wrap this in a try and log if it throws, then continue.


Line 154:       throw new FlumeException("Error closing client", e);
I think we should log errors but continue with shutdown here. Otherwise it will screw up the preconditions in start().


Line 158:   }
I'd suggest throwing at the very end if we got any exceptions while shutting down.


Line 165:         String.format("Missing master addresses. Please specify property '$s'.",
nit: here and elsewhere, checkNotNull() supports substitution syntax: https://google.github.io/guava/releases/snapshot/api/docs/com/google/common/base/Preconditions.html#checkNotNull(T,%20java.lang.String,%20java.lang.Object...)

e.g.:

  Preconditions.checkNotNull(masterAddresses, "Missing master addresses. Please specify property '%s'.", MASTER_ADDRESSES);


http://gerrit.cloudera.org:8080/#/c/4320/1/java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KeyedKuduOperationsProducerTest.java
File java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KeyedKuduOperationsProducerTest.java:

Line 209:     parameters.put(KuduSinkConfigurationConstants.PRODUCER, "org.apache.kudu.flume.sink.SimpleKeyedKuduOperationsProducer");
nit: please wrap this long line. also prefer:

  parameters.put(KuduSinkConfigurationConstants.PRODUCER,
      SimpleKeyedKuduOperationsProducer.class.getName());

If you do that when possible, IDE refactorings and shading have a better chance of working.


-- 
To view, visit http://gerrit.cloudera.org:8080/4320
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I357df8cac7daa6ce105f9568cc3af09697032eb6
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Ara Ebrahimi <ar...@argyledata.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] API and style improvements to the Kudu Flume Sink

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.

Mike Percy has submitted this change and it was merged.

Change subject: API and style improvements to the Kudu Flume Sink
......................................................................

API and style improvements to the Kudu Flume Sink

Change-Id: I357df8cac7daa6ce105f9568cc3af09697032eb6
Reviewed-on: http://gerrit.cloudera.org:8080/4320
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy <mp...@apache.org>
---
M docs/release_notes.adoc
R java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducer.java
D java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduEventProducer.java
A java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduOperationsProducer.java
M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSink.java
M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSinkConfigurationConstants.java
D java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/SimpleKeyedKuduEventProducer.java
A java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/SimpleKeyedKuduOperationsProducer.java
R java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/SimpleKuduOperationsProducer.java
R java/kudu-flume-sink/src/test/avro/testAvroKuduOperationsProducer.avsc
R java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducerTest.java
R java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KeyedKuduOperationsProducerTest.java
12 files changed, 375 insertions(+), 321 deletions(-)

Approvals:
Mike Percy: Looks good to me, approved
Kudu Jenkins: Verified

--
To view, visit http://gerrit.cloudera.org:8080/4320
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I357df8cac7daa6ce105f9568cc3af09697032eb6
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Ara Ebrahimi <ar...@argyledata.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] API and style improvements to the Kudu Flume Sink

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.

Will Berkeley has posted comments on this change.

Change subject: API and style improvements to the Kudu Flume Sink
......................................................................


Patch Set 1:

(8 comments)

Do we also want something in the release notes about the API change + AvroKuduOperationsProducer?

http://gerrit.cloudera.org:8080/#/c/4320/1/java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducer.java
File java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducer.java:

Line 243:               String.format("Failed to coerce value for column `%s` to type %s",
> nit: if we are going to quote, let's use single-quotes, not backquotes here
Done


http://gerrit.cloudera.org:8080/#/c/4320/1/java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduOperationsProducer.java
File java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduOperationsProducer.java:

Line 36: public interface KuduOperationsProducer extends Configurable {
> How about also: extends AutoCloseable
Done


http://gerrit.cloudera.org:8080/#/c/4320/1/java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSink.java
File java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSink.java:

Line 101:   private KuduOperationsProducer eventProducer;
> Shouldn't this also be called operationsProducer instead of eventProducer?
Done


Line 145:     eventProducer.close();
> I think we should wrap this in a try and log if it throws, then continue.
Done


Line 154:       throw new FlumeException("Error closing client", e);
> I think we should log errors but continue with shutdown here. Otherwise it 
Done


Line 158:   }
> I'd suggest throwing at the very end if we got any exceptions while shuttin
Done


Line 165:         String.format("Missing master addresses. Please specify property '$s'.",
> nit: here and elsewhere, checkNotNull() supports substitution syntax: https
Done


http://gerrit.cloudera.org:8080/#/c/4320/1/java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KeyedKuduOperationsProducerTest.java
File java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KeyedKuduOperationsProducerTest.java:

Line 209:     parameters.put(KuduSinkConfigurationConstants.PRODUCER, "org.apache.kudu.flume.sink.SimpleKeyedKuduOperationsProducer");
> nit: please wrap this long line. also prefer:
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/4320
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I357df8cac7daa6ce105f9568cc3af09697032eb6
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Ara Ebrahimi <ar...@argyledata.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes

[kudu-CR] API and style improvements to the Kudu Flume Sink

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.

Hello Kudu Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/4320

to look at the new patch set (#3).

Change subject: API and style improvements to the Kudu Flume Sink
......................................................................

API and style improvements to the Kudu Flume Sink

Change-Id: I357df8cac7daa6ce105f9568cc3af09697032eb6
---
M docs/release_notes.adoc
R java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducer.java
D java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduEventProducer.java
A java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduOperationsProducer.java
M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSink.java
M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSinkConfigurationConstants.java
D java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/SimpleKeyedKuduEventProducer.java
A java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/SimpleKeyedKuduOperationsProducer.java
R java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/SimpleKuduOperationsProducer.java
R java/kudu-flume-sink/src/test/avro/testAvroKuduOperationsProducer.avsc
R java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducerTest.java
R java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KeyedKuduOperationsProducerTest.java
12 files changed, 375 insertions(+), 321 deletions(-)

git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/4320/3
--
To view, visit http://gerrit.cloudera.org:8080/4320
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I357df8cac7daa6ce105f9568cc3af09697032eb6
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Ara Ebrahimi <ar...@argyledata.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] API and style improvements to the Kudu Flume Sink

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.

Hello Kudu Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/4320

to look at the new patch set (#2).

Change subject: API and style improvements to the Kudu Flume Sink
......................................................................

API and style improvements to the Kudu Flume Sink

git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/4320/2
--
To view, visit http://gerrit.cloudera.org:8080/4320
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I357df8cac7daa6ce105f9568cc3af09697032eb6
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Ara Ebrahimi <ar...@argyledata.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] API and style improvements to the Kudu Flume Sink

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.

Mike Percy has posted comments on this change.

Change subject: API and style improvements to the Kudu Flume Sink
......................................................................


Patch Set 3:

OK perfect

-- 
To view, visit http://gerrit.cloudera.org:8080/4320
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I357df8cac7daa6ce105f9568cc3af09697032eb6
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Ara Ebrahimi <ar...@argyledata.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: No

[kudu-CR] API and style improvements to the Kudu Flume Sink

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.

Will Berkeley has posted comments on this change.

Change subject: API and style improvements to the Kudu Flume Sink
......................................................................


Patch Set 3:

> > Do we also want something in the release notes about the API
 > change
 > > + AvroKuduOperationsProducer?
 > 
 > Looks good. Yes, please add to the release notes for 1.0.0 in the
 > Incompatibility section as well as new features.
 > 
 > I think the AvroKuduOperationsProducer would also make a good topic
 > for a blog post, if / when you have time, because it makes
 > integrating with Kudu so easy.

Added to release notes for 1.0.0. I think I can find time for a blog post. But also don't forget about RegexpKuduEventProducer, which I need to rebase and refactor as RegexpKuduOperationsProducer. Will try to do that later today.

-- 
To view, visit http://gerrit.cloudera.org:8080/4320
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I357df8cac7daa6ce105f9568cc3af09697032eb6
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Ara Ebrahimi <ar...@argyledata.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: No

[kudu-CR] API and style improvements to the Kudu Flume Sink

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.

Will Berkeley has posted comments on this change.

Change subject: API and style improvements to the Kudu Flume Sink
......................................................................


Patch Set 3:

> > don't forget about RegexpKuduEventProducer,
 > > which I need to rebase and refactor as RegexpKuduOperationsProducer.
 > > Will try to do that later today.
 > 
 > That doesn't block this patch, does it? This can be committed and
 > you can rebase it onto master.

That's the plan.

-- 
To view, visit http://gerrit.cloudera.org:8080/4320
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I357df8cac7daa6ce105f9568cc3af09697032eb6
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Ara Ebrahimi <ar...@argyledata.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: No

[kudu-CR] API and style improvements to the Kudu Flume Sink

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.

Kudu Jenkins has posted comments on this change.

Change subject: API and style improvements to the Kudu Flume Sink
......................................................................


Patch Set 2:

Build Started http://104.196.14.100/job/kudu-gerrit/3282/

-- 
To view, visit http://gerrit.cloudera.org:8080/4320
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I357df8cac7daa6ce105f9568cc3af09697032eb6
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Ara Ebrahimi <ar...@argyledata.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: No