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 WrappedMutableMapBase extend Serializable #2784

Merged

Conversation

ceedubs
Copy link
Contributor

@ceedubs ceedubs commented Apr 10, 2019

We are running into Spark Task not serializable issues when a closure
that executes on a Spark executor node involves a Map that is created
via running foldMap on a List. This commit makes the
WrappedMutableMap hierarchy extend Serializable and chex that the
cerealization works (this test failed before extending Serializable).

We are running into Spark `Task not serializable` issues when a closure
that executes on a Spark executor node involves a `Map` that is created
via running `foldMap` on a `List`. This commit makes the
`WrappedMutableMap` hierarchy extend `Serializable` and chex that the
cerealization works (this test failed before extending `Serializable`).
@codecov-io
Copy link

codecov-io commented Apr 10, 2019

Codecov Report

Merging #2784 into master will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2784      +/-   ##
==========================================
- Coverage   94.68%   94.65%   -0.03%     
==========================================
  Files         365      365              
  Lines        6869     6869              
  Branches      293      293              
==========================================
- Hits         6504     6502       -2     
- Misses        365      367       +2
Impacted Files Coverage Δ
...12-/cats/kernel/compat/WrappedMutableMapBase.scala 0% <ø> (ø) ⬆️
testkit/src/main/scala/cats/tests/Helpers.scala 96% <0%> (-4%) ⬇️
...rc/main/scala/cats/kernel/instances/function.scala 96.96% <0%> (-3.04%) ⬇️

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 d3b6ee7...e1ff55d. Read the comment docs.

Copy link
Contributor

@kailuowang kailuowang left a comment

Choose a reason for hiding this comment

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

Thanks!

@kailuowang kailuowang merged commit b749ca9 into typelevel:master Apr 10, 2019
@rossabaker
Copy link
Member

Are we all just going to pretend we didn't see those puns?

@ceedubs ceedubs deleted the cerealizable-wrapped-mutable-map branch April 11, 2019 02:47
@kailuowang kailuowang added this to the 2.0.0-M1 milestone Apr 18, 2019
@kailuowang kailuowang added the bug label Apr 18, 2019
rossabaker pushed a commit to rossabaker/cats that referenced this pull request May 26, 2019
We are running into Spark `Task not serializable` issues when a closure
that executes on a Spark executor node involves a `Map` that is created
via running `foldMap` on a `List`. This commit makes the
`WrappedMutableMap` hierarchy extend `Serializable` and chex that the
cerealization works (this test failed before extending `Serializable`).
rossabaker added a commit that referenced this pull request Jun 3, 2019
* Change MonadErrorOps#reject so it no longer runs effects twice (#2810)

* Add regression test for MonadErrorOps bug

* Change MonadErrorOps#reject so it no longer re-runs the effect it is called on. Fixes #2809

* Fix Order.max and Oder.min description comments (#2842)

Changed description to better fit their implentation

* Make WrappedMutableMapBase extend Serializable (#2784)

We are running into Spark `Task not serializable` issues when a closure
that executes on a Spark executor node involves a `Map` that is created
via running `foldMap` on a `List`. This commit makes the
`WrappedMutableMap` hierarchy extend `Serializable` and chex that the
cerealization works (this test failed before extending `Serializable`).

* Optimize productR in Apply (#2728)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants