[
https://issues.apache.org/jira/browse/HDFS-12385?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16159158#comment-16159158
]
Xiaoyu Yao commented on HDFS-12385:
-----------------------------------
Thanks [~nandakumar131] for working on this. The patch looks good to me
overall. I just have a few comments below.
*ClientProtocol.java*
Line 38-41: NIT: suggest edit:
The protocol used for communication is
determined by the implementation class specified by
property <code>ozone.client.protocol</code>. The build-in implementation
includes: org.apache.hadoop.ozone.client.rest.RestProtocol for REST and
org.apache.hadoop.ozone.client.rpc.RpcProtocol for RPC.
Please fix the java doc for the protocol interface
Line 82-83: the @return and @throws are incorrect.
Line 150: missing @param for addAcls
Line 163: missing @param for removeAcls
Line 170: missing @param for versioning
Line 180: missing @param for storageType
Line 200: missing @param for bucketName
Line 219: @return is incorrect
Line 229: missing @param for keyName
Line 233 @return is incorrect
Line 241: missing @param for keyName
Line 250: missing @param for parameters
...
*OzoneClient.java*
I understand that we want to have a proxy handle inside the
OzoneVolume/OzoneBucket/OzoneKey object so that client can simply use volume
object to create bucket, etc. transparently.
I like the document and example between Line 33-65, can you add one more entity
on the right (ClientProtocol) and the referent to it from
Store/Volume/Bucket/Key?
One question I have is how do we expect the client to handle the dangling
reference inside the volume/bucket/key when the AutoClosable OzoneClient is
closed? Do we need reference counting for the proxy handle before the
OzoneClient can be closed?
*RestProtocol.java*
Suggest change the name of the client from "RestProtocol" to "RestClient"
Line 46: NIT: "Ozone client REST protocol implementation. It
uses REST protocol to connect to Ozone Handler that executes client
calls"
Line 80: ozone.rest.servers?
Line 83: Will"localhost" will work with non-standalone case?
Do we have an open ticket for implement the RestProtol class?
*RpcProtocol.java*
Suggest change the name to RpcClient instead of RpcProtocol
*TestOzoneRpcClient.java*
Line 81: we will need to close ozClient to avoid leaking.
> Ozone: OzoneClient: Refactoring OzoneClient API
> -----------------------------------------------
>
> Key: HDFS-12385
> URL: https://issues.apache.org/jira/browse/HDFS-12385
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: ozone
> Reporter: Nandakumar
> Assignee: Nandakumar
> Labels: ozoneMerge
> Attachments: HDFS-12385-HDFS-7240.000.patch, OzoneClient.pdf
>
>
> This jira is for refactoring {{OzoneClient}} API. [^OzoneClient.pdf] will
> give an idea on how the API will look.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
