Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make XML parsing error message more explicit #97

Merged
merged 2 commits into from
May 24, 2019

Conversation

fengnanli
Copy link
Contributor

There are two points in this pr:

  1. when parsing the xml INodeSection, the state machine actually requires that must be the ending of a line, otherwise other states will kick in thus will result in errors.

  2. expose the actual line under parsing in the exceptions for easier debugging, otherwise there is no clue what happened.

@xkrogen
Copy link
Collaborator

xkrogen commented May 24, 2019

@fengnanli , can you look into the test failure? It looks legitimate.

@fengnanli
Copy link
Contributor Author

fengnanli commented May 24, 2019

@xkrogen so the parsing failed at this line: <directory><parent>16386</parent><inode>16390</inode></directory>
This line actually is not for the block report part <INodeSection> and it is in <INodeDirectorySection>. I will undo the change for the parser for </inode>, and file another pr to let the blockgen stop early when it finishes the INodeSection.

FYI, hdfs oiv 2.8.2 doesn't generate <inode> tag in the dir anymore, it is <child> instead.

This should be revisited in the future
@fengnanli fengnanli changed the title Make XML parsing error message more explicit; use endWith for inode c… Make XML parsing error message more explicit May 24, 2019
@xkrogen
Copy link
Collaborator

xkrogen commented May 24, 2019

Interesting... Thanks for the info! I would definitely welcome a change which supports the new-style <child> alongside the old-style <inode>. Annoying that it switched. I'll approve this change in the mean time.

@xkrogen xkrogen merged commit 2d2591e into linkedin:master May 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants