[
https://issues.apache.org/jira/browse/FLUME-989?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13225366#comment-13225366
]
[email protected] commented on FLUME-989:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4047/#review5734
-----------------------------------------------------------
Thanks for the patch Mike. The changes are coming together nicely. I do have
some feedback that follows.
flume-ng-sdk/src/main/java/org/apache/flume/api/client/RpcCallback.java
<https://reviews.apache.org/r/4047/#comment12482>
Unused interface, should be removed.
flume-ng-sdk/src/main/java/org/apache/flume/api/client/RpcClient.java
<https://reviews.apache.org/r/4047/#comment12473>
I suggest removing the FlumeRemoteException and FlumeTimeoutException in
favor of EventDeliveryException as that is what the client is concerned with
from user's perspective.
Exposing these finer grain exceptions allow the creation of more
sophisticated clients which runs contrary to the goal of keeping the client as
simple to code as possible. Once we have concrete use-case from the field where
exposing this semantic is necessary, we can easily introduce it at that time.
Similarly we should remove the RpcResponse altogether. For blocking calls,
the best way to communicate failure is via the exception and any more state
insight that RpcResponse gives will add to the complexity of the client layer.
flume-ng-sdk/src/main/java/org/apache/flume/api/client/RpcClient.java
<https://reviews.apache.org/r/4047/#comment12474>
This should be removed and the functionality should be managed within the
client implementation via configuration.
flume-ng-sdk/src/main/java/org/apache/flume/api/client/RpcClient.java
<https://reviews.apache.org/r/4047/#comment12475>
Same here as the previous comment.
flume-ng-sdk/src/main/java/org/apache/flume/api/client/RpcClient.java
<https://reviews.apache.org/r/4047/#comment12481>
no such method #appendBatch(List, RpcCallback)
flume-ng-sdk/src/main/java/org/apache/flume/api/client/RpcClient.java
<https://reviews.apache.org/r/4047/#comment12471>
nit: The regular idiom is to check for positive and not negative. So it
would make sense for this method to be isActive() instead.
flume-ng-sdk/src/main/java/org/apache/flume/api/client/RpcClientBuilder.java
<https://reviews.apache.org/r/4047/#comment12480>
This should not be exposed but read via configuration.
flume-ng-sdk/src/main/java/org/apache/flume/api/client/RpcClientBuilder.java
<https://reviews.apache.org/r/4047/#comment12476>
These checks seem redundant since the builder will check them again anyway.
Finally, please include some direct tests that exercise this API.
- Arvind
On 2012-03-08 11:14:53, Mike Percy wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/4047/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2012-03-08 11:14:53)
bq.
bq.
bq. Review request for Flume.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. Seeking early feedback on some additional APIs to make integrating with
Flume 1.x easier.
bq.
bq. Added the following APIs:
bq. - AvroClient: Friendly Java interface around the Avro API
bq. - AvroClientBuilder: Builder class to allow easy extension of AvroClient
capabilities in the future (i.e. SSL)
bq. - DefaultAvroClient: Implementation of the AvroClient interface
bq.
bq. Created this stuff in a flume-ng-sdk Maven submodule and moved the Event
interface to that submodule. flume-ng-core depends on flume-ng-sdk.
bq.
bq. I also modified AvroSink to use the AvroClient API instead of the bare
AvroSourceProtocol API directly.
bq.
bq.
bq. This addresses bug FLUME-989.
bq. https://issues.apache.org/jira/browse/FLUME-989
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. flume-ng-clients/flume-ng-log4jappender/pom.xml 9dd31bd
bq.
flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java
97f2b9e
bq. flume-ng-core/pom.xml fe6ce0b
bq. flume-ng-core/src/main/avro/flume.avdl 40da3ef
bq. flume-ng-core/src/main/java/org/apache/flume/Event.java a017705
bq. flume-ng-core/src/main/java/org/apache/flume/FlumeException.java eab5b3d
bq.
flume-ng-core/src/main/java/org/apache/flume/client/avro/AvroCLIClient.java
195ba79
bq. flume-ng-core/src/main/java/org/apache/flume/event/EventBuilder.java
5d8c3b3
bq. flume-ng-core/src/main/java/org/apache/flume/event/SimpleEvent.java
e0c3b45
bq. flume-ng-core/src/main/java/org/apache/flume/sink/AvroSink.java 7386d06
bq. flume-ng-core/src/test/java/org/apache/flume/sink/TestAvroSink.java
467785f
bq. flume-ng-sdk/pom.xml PRE-CREATION
bq. flume-ng-sdk/src/main/avro/flume.avdl PRE-CREATION
bq. flume-ng-sdk/src/main/java/org/apache/flume/Event.java PRE-CREATION
bq. flume-ng-sdk/src/main/java/org/apache/flume/FlumeException.java
PRE-CREATION
bq. flume-ng-sdk/src/main/java/org/apache/flume/FlumeRemoteException.java
PRE-CREATION
bq. flume-ng-sdk/src/main/java/org/apache/flume/FlumeTimeoutException.java
PRE-CREATION
bq.
flume-ng-sdk/src/main/java/org/apache/flume/api/client/NettyAvroRpcClient.java
PRE-CREATION
bq. flume-ng-sdk/src/main/java/org/apache/flume/api/client/RpcCallback.java
PRE-CREATION
bq. flume-ng-sdk/src/main/java/org/apache/flume/api/client/RpcClient.java
PRE-CREATION
bq.
flume-ng-sdk/src/main/java/org/apache/flume/api/client/RpcClientBuilder.java
PRE-CREATION
bq. flume-ng-sdk/src/main/java/org/apache/flume/api/client/RpcResponse.java
PRE-CREATION
bq. flume-ng-sdk/src/main/java/org/apache/flume/event/EventBuilder.java
PRE-CREATION
bq. flume-ng-sdk/src/main/java/org/apache/flume/event/SimpleEvent.java
PRE-CREATION
bq. pom.xml d785762
bq.
bq. Diff: https://reviews.apache.org/r/4047/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq.
bq. Thanks,
bq.
bq. Mike
bq.
bq.
> Factor Flume Avro RPC interfaces out into separate Client SDK
> -------------------------------------------------------------
>
> Key: FLUME-989
> URL: https://issues.apache.org/jira/browse/FLUME-989
> Project: Flume
> Issue Type: Sub-task
> Affects Versions: v1.0.0
> Reporter: Mike Percy
> Assignee: Mike Percy
> Fix For: v1.1.0
>
>
> Factor out the network RPC APIs so that there is a concise API boundary
> between user and developer APIs.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
