[ 
https://issues.apache.org/jira/browse/HBASE-17315?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15805752#comment-15805752
 ] 

Enis Soztutar commented on HBASE-17315:
---------------------------------------

A typo here: 
{code}
- * Licensed to the Apache Software Foundation (ASF) under one
+ w * Licensed to the Apache Software Foundation (ASF) under one
{code}

For both of these constructors: 
{code}
+Client::Client(const std::string &zk_quorum,
+               const std::shared_ptr<hbase::optional<hbase::Configuration>> 
&conf)
{code}

The zk_quorum will be read from the Configuration. No need for applications to 
pass it anymore. So this: 
{code}
auto zk_quorum =
+      (conf) ? conf->Get("hbase.zookeeper.quorum", "localhost:2181")
{code} 
should happen internally within the LocationCache, or Client. Plus, the 
{{hbase.zookeeper.quorum}} should be extracted as a constant somewhere. 

You should not have a shared_ptr to an optional value like this: 
{code}
+  std::shared_ptr<hbase::optional<hbase::Configuration>> conf_ =
{code}
The configuration parser returns an optional, but for our purposes, if the 
configuration cannot be parsed from optional, then the Client should raise an 
error back.  

For these (and maybe others): 
{code}
std::unique_ptr<Result> Table::Get(const hbase::Get &get)
std::unique_ptr<hbase::Table> Table(const TableName &table_name);
{code}

Why are we returning via unique_ptr? Isnt' move semantics automatically kick in 
via optimization when we return by value? 

Remove this:
{code}
+  // client_->Close();
{code}

Why are we catching exception, and returning nullptr? 
{code}
+  catch (const std::runtime_error &rex) {
+    LOG(ERROR) << "Caught exception while performing Table::Get() :-" << 
rex.what();
+    return nullptr;
+  }
{code} 
we should raise the exception, back to the application. 


> [C++] HBase Client and Table Implementation
> -------------------------------------------
>
>                 Key: HBASE-17315
>                 URL: https://issues.apache.org/jira/browse/HBASE-17315
>             Project: HBase
>          Issue Type: Sub-task
>            Reporter: Sudeep Sunthankar
>            Assignee: Sudeep Sunthankar
>         Attachments: HBASE-17315.HBASE-14850.v1.patch, 
> HBASE-17315.HBASE-14850.v2.patch, HBASE-17315.HBASE-14850.v3.patch, 
> HBASE-17315.HBASE-14850.v4.patch
>
>
> Consists of Client and Table implementation which will be used to call the 
> corresponding client methods i.e Get, Gets, Scan etc. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to