Skip to content
This repository was archived by the owner on Mar 24, 2025. It is now read-only.

Conversation

@pwoody
Copy link

@pwoody pwoody commented Feb 9, 2018

Hadoop added a woodstox dependency in https://issues.apache.org/jira/browse/HADOOP-14501. When using versions of Hadoop with that dependency, WstxOutputFactory will get loaded for XMLOutputFactory.newInstance. The resulting streamwriter will ensure that a client does not write two root elements, which causes the current code to error out.

This PR makes it such that the writer function will start the root tag before writing rows.

@codecov-io
Copy link

codecov-io commented Feb 9, 2018

Codecov Report

Merging #282 into master will decrease coverage by 0.07%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #282      +/-   ##
==========================================
- Coverage   88.52%   88.44%   -0.08%     
==========================================
  Files          14       14              
  Lines         732      727       -5     
  Branches      101       97       -4     
==========================================
- Hits          648      643       -5     
  Misses         84       84
Impacted Files Coverage Δ
.../scala/com/databricks/spark/xml/util/XmlFile.scala 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3e5d972...4d92847. Read the comment docs.

@pwoody
Copy link
Author

pwoody commented Feb 9, 2018

@HyukjinKwon let me know what you think. Thanks!

@HyukjinKwon
Copy link
Member

Wow, thanks for fixing it @pwoody. Seems fine. Let me merge this one after another double check within few days.

var lastRow: Boolean = true

override def hasNext: Boolean = iter.hasNext || firstRow || lastRow

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pwoody, mind if I ask to remove val startElement = s"<${options.rootTag}>" and val endElement = s"</${options.rootTag}>" above? Seems unused.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, done.

@pwoody pwoody force-pushed the pw/multipleRootFix branch from 171df25 to 4d92847 Compare February 10, 2018 21:03
Copy link
Member

@HyukjinKwon HyukjinKwon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks @pwoody.

HyukjinKwon pushed a commit that referenced this pull request Apr 2, 2018
Hadoop added a woodstox dependency in https://issues.apache.org/jira/browse/HADOOP-14501. When using versions of Hadoop with that dependency, WstxOutputFactory will get loaded for XMLOutputFactory.newInstance. The resulting streamwriter will ensure that a client does not write two root elements, which causes the current code to error out.

This PR makes it such that the writer function will start the root tag before writing rows.

Author: Patrick Woody <[email protected]>

Closes #282 from pwoody/pw/multipleRootFix.

(cherry picked from commit 3b09617)
Signed-off-by: hyukjinkwon <[email protected]>
@HyukjinKwon
Copy link
Member

Merged to master and branch-0.4!

@pwoody pwoody deleted the pw/multipleRootFix branch April 2, 2018 15:29
@pwoody
Copy link
Author

pwoody commented Apr 2, 2018

Thanks @HyukjinKwon - what is the release cadence for a 0.4.x release?

@HyukjinKwon
Copy link
Member

Will make a release soon within next week (after sorting out the current open PRs). Does that work for you too?

@pwoody
Copy link
Author

pwoody commented Apr 3, 2018 via email

@HyukjinKwon
Copy link
Member

Hey, @pwoody. I'm really sorry that the release has been postponed. 0.5.0 is released now. Thank you for your work .. !

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants