Skip to content

Add InvalidSourceTap to catch all cases for no good path.#1458

Merged
rubanm merged 6 commits intodevelopfrom
klin/invalid/sourcetap
Jan 6, 2016
Merged

Add InvalidSourceTap to catch all cases for no good path.#1458
rubanm merged 6 commits intodevelopfrom
klin/invalid/sourcetap

Conversation

@reconditesea
Copy link
Contributor

To close #1454.

@rubanm
Copy link
Contributor

rubanm commented Dec 17, 2015

We have an internal version of this Tap with the same name. It handles some of the quirks required to work with cascading's MultiInputFormat and also unique identifiers. If you could consolidate the two, that'd be great.

Copy link
Contributor

Choose a reason for hiding this comment

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

This can extend just SourceTap.

@reconditesea
Copy link
Contributor Author

@rubanm Didn't know we already have an internal version. Will take a look and consolidate them.

@ulyssepence
Copy link
Contributor

Do we have a test yet that hits this code path in FileSource?

@reconditesea
Copy link
Contributor Author

@benpence Added a unit-test. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

goodHdfsPaths will return Nil in this case right? We need to pass hdfsPaths here I think so that openForRead throws with the right message. Can you also update the comment above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch!

@rubanm
Copy link
Contributor

rubanm commented Dec 23, 2015

LGTM. @johnynek could you take a look as well? Thanks.

@reconditesea
Copy link
Contributor Author

@johnynek @rubanm Any suggestions on this PR? Thanks and happy holidays!

@reconditesea
Copy link
Contributor Author

Gentle ping.

@rubanm
Copy link
Contributor

rubanm commented Jan 6, 2016

LGTM merging this. @reconditesea thanks!

The internal version of this at Twitter has been working fine so I don't foresee any problems. Also with this change, opening an Iterator via toIterator on an invalid source will throw InvalidSourceException (instead of returning an iterator on an invalid hdfs datapath, which in turn, throws hadoop's InvalidInputException).

rubanm added a commit that referenced this pull request Jan 6, 2016
Add InvalidSourceTap to catch all cases for no good path.
@rubanm rubanm merged commit c41a0d5 into develop Jan 6, 2016
@johnynek johnynek deleted the klin/invalid/sourcetap branch January 7, 2016 00:34
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.

Add separate InvalidSourceTap for dealing with no valid paths in FileSource

3 participants