[
https://issues.apache.org/jira/browse/HDFS-9117?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14978590#comment-14978590
]
Bob Hansen commented on HDFS-9117:
----------------------------------
bq. 1. It's beneficial to further separate the patch, such as substituting
variables, and all the TODO items to a separate jira.
Removed the variable substitution, and re-cast the TODOs as a comment capturing
"Here's the things you might expect this class to do, that it doesn't do yet"
bq. 2. The code should not use C++ exceptions.
I think the code was cleaner using the std::string parsers (which had
exceptions that were caught), but I moved them to the strtoXXX functions.
bq. 3. That current interface is insufficient in terms of not every
configuration has a default value.
It's a good point; I was unaware that we had use cases where there would be no
default. Unfortunately, we don't have boost::optional available, so the API is
a bit muddier now.
{quote}
4. It's much cleaner to implement the final attribute with priority. The
configuration follow the follow rules:
The value reads later overrides the earlier values.
Values that are tagged as final overrides the previous value.
It essentially defines a total order which can be mapped to an integer.
{quote}
The implementation follows the rules outlined (and all the tests over them). I
implemented it as a final flag on all the values which prevents future
resources from changing it, which seems a very natural implementation. Is the
current implementation wrong in any way?
bq. 6. I suggest leaving environment substitutions and other platform-specific
functionality out (at least for now).
It has been moved over with the other substitutions. It's implemented, tested,
and part of the spec of the Java config files, so I don't see any reason to
excise it.
bq. 7. I suggest putting the code along with the libhdfs compatibility layer.
I disagree; the code is required for a pure C++ implementation that satisfies
the use case of "Read the local config and connect to HDFS." The libhdfs
compatability layer should be for the glue for the C API.
> Config file reader / options classes for libhdfs++
> --------------------------------------------------
>
> Key: HDFS-9117
> URL: https://issues.apache.org/jira/browse/HDFS-9117
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: hdfs-client
> Affects Versions: HDFS-8707
> Reporter: Bob Hansen
> Assignee: Bob Hansen
> Attachments: HDFS-9117.HDFS-8707.001.patch,
> HDFS-9117.HDFS-8707.002.patch, HDFS-9117.HDFS-8707.003.patch,
> HDFS-9117.HDFS-8707.004.patch, HDFS-9117.HDFS-8707.005.patch,
> HDFS-9117.HDFS-8707.006.patch, HDFS-9117.HDFS-8707.008.patch,
> HDFS-9117.HDFS-9288.007.patch
>
>
> For environmental compatability with HDFS installations, libhdfs++ should be
> able to read the configurations from Hadoop XML files and behave in line with
> the Java implementation.
> Most notably, machine names and ports should be readable from Hadoop XML
> configuration files.
> Similarly, an internal Options architecture for libhdfs++ should be developed
> to efficiently transport the configuration information within the system.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
