Skip to content

Conversation

@HyukjinKwon
Copy link
Member

@HyukjinKwon HyukjinKwon commented May 3, 2019

This PR proposes to add an env CLOUDPICKLE_DEFAULT_PROTOCOL for users to set the default protocol.

Few issues were found after Spark side upgrades cloudpickle from 0.4 to 0.8. Other issues were fixed but one issue is (I suspect) in Pyrolite or at least it's related. Spark uses Pyrolite to unpickle Python objects in JVM and looks Pyrolite has a bug when to unpickle lists (see SPARK-27612).

For clarification, I want this env as an internal purpose for now and see if this actually brings some overhead for dev. If it doesn't, I would like to keep this env. If it does, let's drop this env.

@HyukjinKwon HyukjinKwon force-pushed the env-default-protocol branch from e944d94 to faab38e Compare May 3, 2019 01:16
@codecov
Copy link

codecov bot commented May 3, 2019

Codecov Report

Merging #265 into master will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #265      +/-   ##
==========================================
+ Coverage   88.22%   88.24%   +0.02%     
==========================================
  Files           1        1              
  Lines         552      553       +1     
  Branches      112      112              
==========================================
+ Hits          487      488       +1     
  Misses         42       42              
  Partials       23       23
Impacted Files Coverage Δ
cloudpickle/cloudpickle.py 88.24% <100%> (+0.02%) ⬆️

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 6cb4718...faab38e. Read the comment docs.

@HyukjinKwon
Copy link
Member Author

Okie.Spark side worked round the bug and increased protocol back. Let me leave this closed.

@HyukjinKwon HyukjinKwon closed this May 4, 2019
@ogrisel
Copy link
Contributor

ogrisel commented May 15, 2019

It's not a big overhead so let us know if you need to add it back.

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.

2 participants