Skip to content
This repository has been archived by the owner on Jan 20, 2022. It is now read-only.

remove duplicate LookupJoin - use Scalding's. #654

Merged
merged 3 commits into from
Jan 29, 2016
Merged

Conversation

sritchie
Copy link
Collaborator

#596 already did this, but that branch has merge conflicts. Deletes our implementation and tests and relies on the Scalding version of LookupJoin.

The dependencies mentioned in #596 are all complete.

r? @johnynek

@ianoc
Copy link
Collaborator

ianoc commented Jan 29, 2016

Looks like build is failing?

[info] - should compute correct statistics *** FAILED ***
[info]   java.lang.NullPointerException:
[info]   at com.twitter.summingbird.scalding.ScaldingLaws$$anonfun$1$$anonfun$apply$mcV$sp$12.apply$mcV$sp(ScaldingLaws.scala:605)
[info]   at com.twitter.summingbird.scalding.ScaldingLaws$$anonfun$1$$anonfun$apply$mcV$sp$12.apply(ScaldingLaws.scala:573)
[info]   at com.twitter.summingbird.scalding.ScaldingLaws$$anonfun$1$$anonfun$apply$mcV$sp$12.apply(ScaldingLaws.scala:573)
[info]   at org.scalatest.Transformer$$anonfun$apply$1.apply$mcV$sp(Transformer.scala:22)
[info]   at org.scalatest.OutcomeOf$class.outcomeOf(OutcomeOf.scala:85)
[info]   at org.scalatest.OutcomeOf$.outcomeOf(OutcomeOf.scala:104)
[info]   at org.scalatest.Transformer.apply(Transformer.scala:22)
[info]   at org.scalatest.Transformer.apply(Transformer.scala:20)
[info]   at org.scalatest.WordSpecLike$$anon$1.apply(WordSpecLike.scala:953)
[info]   at org.scalatest.Suite$class.withFixture(Suite.scala:1122)

@johnynek
Copy link
Collaborator

weird. This is the line with the NPE:

https://github.com/twitter/summingbird/blob/develop/summingbird-scalding-test/src/test/scala/com/twitter/summingbird/scalding/ScaldingLaws.scala#L605

It looks like that should only happen if the job does not run. Restated.

@sritchie
Copy link
Collaborator Author

Strange. @johnynek @ianoc merge or investigate more?

@ianoc
Copy link
Collaborator

ianoc commented Jan 29, 2016

If its green I think we are good to go. In that maybe we want to check sometime if this re-occurs, but seems unrelated to change

ianoc added a commit that referenced this pull request Jan 29, 2016
remove duplicate LookupJoin - use Scalding's.
@ianoc ianoc merged commit 4385fe5 into develop Jan 29, 2016
@ianoc ianoc deleted the feature/remove_lookup branch January 29, 2016 22:45
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.

5 participants