[
https://issues.apache.org/jira/browse/FLUME-989?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13226015#comment-13226015
]
[email protected] commented on FLUME-989:
-----------------------------------------------------
bq. On 2012-03-08 18:29:17, Arvind Prabhakar wrote:
bq. > flume-ng-sdk/src/main/java/org/apache/flume/api/client/RpcCallback.java,
line 18
bq. > <https://reviews.apache.org/r/4047/diff/2/?file=89096#file89096line18>
bq. >
bq. > Unused interface, should be removed.
Done
bq. On 2012-03-08 18:29:17, Arvind Prabhakar wrote:
bq. > flume-ng-sdk/src/main/java/org/apache/flume/api/client/RpcClient.java,
line 28
bq. > <https://reviews.apache.org/r/4047/diff/2/?file=89097#file89097line28>
bq. >
bq. > I suggest removing the FlumeRemoteException and
FlumeTimeoutException in favor of EventDeliveryException as that is what the
client is concerned with from user's perspective.
bq. >
bq. > 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.
bq. >
bq. > 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.
Done. Note that EventDeliveryException is checked and we should consider making
it inherit from FlumeException for consistency. But I didn't try to make that
change here since it would not be backwards compatible.
bq. On 2012-03-08 18:29:17, Arvind Prabhakar wrote:
bq. > flume-ng-sdk/src/main/java/org/apache/flume/api/client/RpcClient.java,
line 52
bq. > <https://reviews.apache.org/r/4047/diff/2/?file=89097#file89097line52>
bq. >
bq. > This should be removed and the functionality should be managed
within the client implementation via configuration.
I simply removed the functionality from the interface and the default timeouts
are used only for now.
bq. On 2012-03-08 18:29:17, Arvind Prabhakar wrote:
bq. > flume-ng-sdk/src/main/java/org/apache/flume/api/client/RpcClient.java,
line 80
bq. > <https://reviews.apache.org/r/4047/diff/2/?file=89097#file89097line80>
bq. >
bq. > Same here as the previous comment.
bq. >
Done.
bq. On 2012-03-08 18:29:17, Arvind Prabhakar wrote:
bq. > flume-ng-sdk/src/main/java/org/apache/flume/api/client/RpcClient.java,
line 85
bq. > <https://reviews.apache.org/r/4047/diff/2/?file=89097#file89097line85>
bq. >
bq. > no such method #appendBatch(List, RpcCallback)
Thanks, fixed!
bq. On 2012-03-08 18:29:17, Arvind Prabhakar wrote:
bq. > flume-ng-sdk/src/main/java/org/apache/flume/api/client/RpcClient.java,
line 106
bq. > <https://reviews.apache.org/r/4047/diff/2/?file=89097#file89097line106>
bq. >
bq. > nit: The regular idiom is to check for positive and not negative. So
it would make sense for this method to be isActive() instead.
I just removed it, as mentioned above.
bq. On 2012-03-08 18:29:17, Arvind Prabhakar wrote:
bq. >
flume-ng-sdk/src/main/java/org/apache/flume/api/client/RpcClientBuilder.java,
line 90
bq. > <https://reviews.apache.org/r/4047/diff/2/?file=89098#file89098line90>
bq. >
bq. > This should not be exposed but read via configuration.
Hid this functionality for now.
bq. On 2012-03-08 18:29:17, Arvind Prabhakar wrote:
bq. >
flume-ng-sdk/src/main/java/org/apache/flume/api/client/RpcClientBuilder.java,
lines 121-123
bq. > <https://reviews.apache.org/r/4047/diff/2/?file=89098#file89098line121>
bq. >
bq. > These checks seem redundant since the builder will check them again
anyway.
Done.
On 2012-03-08 18:29:17, Mike Percy wrote:
bq. > Finally, please include some direct tests that exercise this API.
Added a bunch of tests that really put the API through its paces.
- Mike
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4047/#review5734
-----------------------------------------------------------
On 2012-03-09 11:14:13, 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-09 11:14:13)
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 5278fc0
bq. flume-ng-core/src/main/java/org/apache/flume/EventDeliveryException.java
1413223
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/main/java/org/apache/flume/source/AvroSource.java
a3f6640
bq. flume-ng-core/src/test/java/org/apache/flume/sink/TestAvroSink.java
467785f
bq. flume-ng-core/src/test/java/org/apache/flume/util/TestEventBuilder.java
7930607
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/EventDeliveryException.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/api/NettyAvroRpcClient.java
PRE-CREATION
bq. flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClient.java
PRE-CREATION
bq. flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.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.
flume-ng-sdk/src/test/java/org/apache/flume/api/NettyAvroRpcTestHelpers.java
PRE-CREATION
bq.
flume-ng-sdk/src/test/java/org/apache/flume/api/TestNettyAvroRpcClient.java
PRE-CREATION
bq.
flume-ng-sdk/src/test/java/org/apache/flume/api/TestRpcClientFactory.java
PRE-CREATION
bq. flume-ng-sdk/src/test/java/org/apache/flume/event/TestEventBuilder.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
