twitter / summingbird

Streaming MapReduce with Scalding and Storm

https://twitter.com/summingbird

Apache License 2.0

2.14k stars 267 forks source link

Support Scalding's 0.18.x branch #769

Closed ttim closed 5 years ago

ttim commented 5 years ago

This is reiteration over https://github.com/twitter/summingbird/pull/762 with correct fix.

I've released newest scalding internally and get green ./sbt clean test of summingbird against this version. Note: to get it I reverted "Fix incompatibilities between storm and heron api" commit.

CLAassistant commented 5 years ago

All committers have signed the CLA.

ttim commented 5 years ago

This PR is against twitter_release branch and will be later (as soon as we release scalding 0.18.x) ported to develop

dieu commented 5 years ago

Can you look on CI?

ttim commented 5 years ago

@dieu just like previous PR this PR isn't going to pass through CI since scalding wasn't released. I tested it locally and tests are green.

johnynek commented 5 years ago

should we release a public RC to test these kinds of things?

ttim commented 5 years ago

@johnynek That totally makes sense, I'll try to release scalding RC today.

johnynek commented 5 years ago

we should publish an RC since I don't think anything is going to change that will break summingbird until the final is released.

PS: this is what we are running at Stripe: https://github.com/stripe/summingbird/tree/v0.10.0-M8-stripe-scalding-18

which works with scalding 0.18

johnynek commented 5 years ago

Change looks good but it looks like failed. On a phone and can’t look now.

ttim commented 5 years ago

@johnynek my bad, this PR was against twitter release branch which doesn't compile because of some classes removed to make it compilable against heron.

Changed the base, should be good now.

ttim commented 5 years ago

Repos were broken as well (http instead of https, twitters maven repo doesn't serve over http anymore)

codecov-io commented 5 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (develop@d5322b3). Click here to learn what that means. The diff coverage is 100%.

@@            Coverage Diff             @@
##             develop     #769   +/-   ##
==========================================
  Coverage           ?   71.49%           
==========================================
  Files              ?      151           
  Lines              ?     3613           
  Branches           ?      209           
==========================================
  Hits               ?     2583           
  Misses             ?     1030           
  Partials           ?        0

Impacted Files	Coverage Δ
...witter/summingbird/scalding/ScaldingPlatform.scala	`77.09% <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 d5322b3...430cef6. Read the comment docs.