hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Enis Soztutar (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-17315) [C++] HBase Client and Table Implementation
Date Tue, 10 Jan 2017 02:01:03 GMT

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

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

Thanks Sudeep for the updated patch. 

No need for optional here: 
{code}
+  std::shared_ptr<hbase::optional<hbase::Configuration>> conf_;
{code}

Change these two to false for now (since they won't be implemented in the initial cut): 
{code}
+  pb_msg->set_client_handles_partials(true);
+  pb_msg->set_client_handles_heartbeats(true);
{code}

Maybe move this class
{code}
/hbase-native-client/core/protobuf_request_builder.cc 
{code}
to the {{serde}} directory and name it request-converter. Also extract out this logic: 
{code}
+  for (auto cell : get_resp->result().cell()) {
+    std::shared_ptr<Cell> pcell =
+        std::make_shared<Cell>(cell.row(), cell.family(), cell.qualifier(), cell.timestamp(),
+                               cell.value(), static_cast<hbase::CellType>(cell.cell_type()));
+    vcells.push_back(pcell);
+  }
+
+  hbase::Result result(vcells, get_resp->result().exists(), get_resp->result().stale(),
+                       get_resp->result().partial());
{code}
into a class called response-converter. 

Also we have discussed internally that retuning via unique_ptr's is better for Table::Get()
and Client::Table methods for now. We can always revisit later.  

Why this? 
{code}
hbase::TestUtil *test_util = new hbase::TestUtil();
..
+  delete test_util;
{code}
Why not unique_ptr, and release()? 

Looking at the way you use configuration for tests, maybe we should do Conf.set(), etc methods,
and maybe do a TestConfigurationLoader or something which is not XML-file based. This way
it will be much easier for future tests. We can do this in a later patch though, no need to
change this now. 

Did you want to enable this assertion? 
{code}
+  // ASSERT_TRUE(table != nullptr) << "Unable to get connection to Table.";
{code}

Otherwise looks pretty good. 

> [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, HBASE-17315.HBASE-14850.v5.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)

Mime
View raw message