[
https://issues.apache.org/jira/browse/HBASE-16489?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15646003#comment-15646003
]
Enis Soztutar commented on HBASE-16489:
---------------------------------------
Thanks for making these changes.
- std::optional is coming in C++17, but we are using 14. Is that the reason we
are using boost::optional? What about std::experimental::optional. Seems like a
safer bet. HDFS patch is using that instead. I tried to google which one is
better, but did not find many references.
{code}
using optional = std::experimental::optional<T>;
{code}
- This should also return an optional value, instead of returning empty value:
+std::string HBaseConfiguration::Get(const std::string &key) const {
- Same thing like these kind of comments:
{code}
// raw.size() > 0 if the property is present.
{code}
Please make it so that internally we never depend on empty strings indicating
NULL / Not Found. In modern code, we should opt for optionals in all applicable
places.
- Let's rename configuration loader to hbase configuration loader, and rename
hbase configuration to configuration.
- For Configuration / ConfigurationLoader, we want to follow the interface of
https://github.com/apache/hadoop/tree/HDFS-8707/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common.
That means unlike the current patch, the configuration should not know
anything about the Search paths, etc. The client API should be something like
this:
{code}
ConfigurationLoader loader;
loader.SetSearchPath("/foo/bar");
Configuration conf = loader.Load();
// create HBase connection from this configuration.
{code}
> Configuration parsing
> ---------------------
>
> Key: HBASE-16489
> URL: https://issues.apache.org/jira/browse/HBASE-16489
> Project: HBase
> Issue Type: Sub-task
> Reporter: Sudeep Sunthankar
> Assignee: Sudeep Sunthankar
> Attachments: HBASE-16489.HBASE-14850.v1.patch,
> HBASE-16489.HBASE-14850.v2.patch, HBASE-16489.HBASE-14850.v3.patch
>
>
> Reading hbase-site.xml is required to read various properties viz.
> zookeeper-quorum, client retires etc. We can either use Apache Xerces or
> Boost libraries.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
