|
By setting XcludeAware to true, we can have xi:include tags in xml file.
XincludeAware requires setNamespaceAware to be true. +1. I think this is a compatible change because it should not affect parsing of existing configuration files.
+1, though there are docs and tests to consider. The other issue is security; it now becomes possible to xinclude from somewhere else, if you aren't careful.
Rama: if you have a unit test for this one, that will be great.
However, i am willing to commit this without a unit test because the code-change is very very minor. Also, in some sense it is an "Improvement" only. so the risk of impacting existing installations is minimal. Rama: let's discuss how we can come up with a simple unit test.
Once the patch becomes a feature, you need the test to make sure it doesn't go away later; for regression testing.
This patch allows including a xml file via the <xi:include> xml tag. I updated Rama's version of the patch to include a unit test as well.
Can some kind soul please review this patch? Thanks.
The current version of this patch changes the syntax of configurations so that they may be nested. Couldn't this extension of the syntax be avoided by using "range-inside", e.g., something like:
<include xmlns="http://www.w3.org/2001/XInclude" href="foo.xml" xpointer="xpointer(range-inside(//configuration))"/> That was the original idea, but xerces currently doesn’t support xpointer() scheme (http://xerces.apache.org/xerces2-j/faq-xinclude.html#faq-8
 ) and got the following error:
hadoop5224.snc1.facebook.com: [Warning] hadoop-site.xml:9:147: SchemeUnsupported: The XPointer scheme 'xpointer' is not supported. > xerces currently doesn't support xpointer() scheme
Sigh. So I wonder if, instead of supporting nested "configuration" tags we might instead support nested "properties" tags, that each contain a set of properties. Or do you think it would be useful to have included files also capable of acting as top-level configuration files? I think it is more powerful for an include file A to be able to include another include file B for a cluster, while at the same time B to be the top level config file for another cluster. I would like to keep it this way, unless you feel strongly otherwise.
As someone who works in CM, I clearly think that being able to include stuff is a handy way to deal with scale. But if you treat every .xml file as toplevel, dont forget to include test cases that
You dont have to support these, property expansion in particular. But get those tests together to make sure that whatever behaviour you have matches expectations. @Steve: Good points. Thaks. I verified that all of the issues you mentioned is actually handled by the Xerces library. It can detect recursive includes. The hadoop Configuration.get() method generates an RuntimeException when this occurs.
I plan to commit this patch to 0.20 and trunk. [exec] +1 overall.
[exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] +1 tests included. The patch appears to include 3 new or modified tests. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 Eclipse classpath. The patch retains Eclipse classpath integrity. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings. I just committed this. Thanks Rama!
re-opening as it appears to raise problems with different XML parsers/JVMs. I'm not sure that 0.20 will want this.
Hi Steve, Hadoop uses the xerces library to parse the XML file. Xerces handles include files well. Can you pl explain what problem you are seeing.
I think myself and someone on the user list has encountered problems which may be caused by the built in JVM's version of Xerces; see
Integrated in Hadoop-trunk #756 (See http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/756/
)
The problems that you saw were fixed in
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 |
|
Improvement
Closed
Minor
Allow Xinclude in hadoop config file
—
<configuration xmlns:xi="http://www.w3.org/2001/XInclude">
...
<xi:include href="test.xml"/>
...
</configuration>