You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by rgbkrk <gi...@git.apache.org> on 2017/06/12 22:34:43 UTC
[GitHub] spark pull request #18282: [SPARK-21070][PYSPARK] Upgrade cloudpickle
GitHub user rgbkrk opened a pull request:
https://github.com/apache/spark/pull/18282
[SPARK-21070][PYSPARK] Upgrade cloudpickle
## What changes were proposed in this pull request?
This brings in fixes and upgrades from the [cloudpickle](https://github.com/cloudpipe/cloudpickle) module. Notable fixes:
* Import submodules accessed by pickled functions (https://github.com/cloudpipe/cloudpickle/pull/80)
* Support recursive functions inside closures (https://github.com/cloudpipe/cloudpickle/pull/89, https://github.com/cloudpipe/cloudpickle/pull/90)
* Fix ResourceWarnings and DeprecationWarnings (https://github.com/cloudpipe/cloudpickle/pull/88)
* Assume modules with __file__ attribute are not dynamic (https://github.com/cloudpipe/cloudpickle/pull/85)
* Make cloudpickle Python 3.6 compatible (https://github.com/cloudpipe/cloudpickle/pull/72)
* Allow pickling of builtin methods (https://github.com/cloudpipe/cloudpickle/pull/57)
* Add ability to pickle dynamically created modules (https://github.com/cloudpipe/cloudpickle/pull/52)
* Support method descriptor (https://github.com/cloudpipe/cloudpickle/pull/46)
* No more pickling of closed files, was broken on Python 3 (https://github.com/cloudpipe/cloudpickle/pull/32)
## How was this patch tested?
The [test suite for cloudpickle](https://github.com/cloudpipe/cloudpickle/tree/master/tests) tests the internal implementation. There are integration tests here in PySpark that use the `CloudPickleSerializer`.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/rgbkrk/spark cloudpickle-upgrades
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/18282.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #18282
----
commit b84222b6ea660ce5ec1aedfee50e297b436eb824
Author: Kyle Kelley <rg...@gmail.com>
Date: 2017-06-12T22:19:29Z
[SPARK-21070][PYSPARK] Upgrade cloudpickle
This brings in fixes and upgrades from the [cloudpickle](https://github.com/cloudpipe/cloudpickle) module, notably:
* Import submodules accessed by pickled functions (https://github.com/cloudpipe/cloudpickle/pull/80)
* Support recursive functions inside closures (https://github.com/cloudpipe/cloudpickle/pull/89, https://github.com/cloudpipe/cloudpickle/pull/90)
* Fix ResourceWarnings and DeprecationWarnings (https://github.com/cloudpipe/cloudpickle/pull/88)
* Assume modules with __file__ attribute are not dynamic (https://github.com/cloudpipe/cloudpickle/pull/85)
* Make cloudpickle Python 3.6 compatible (https://github.com/cloudpipe/cloudpickle/pull/72)
* Allow pickling of builtin methods (https://github.com/cloudpipe/cloudpickle/pull/57)
* Add ability to pickle dynamically created modules (https://github.com/cloudpipe/cloudpickle/pull/52)
* Support method descriptor (https://github.com/cloudpipe/cloudpickle/pull/46)
* No more pickling of closed files, was broken on Python 3 (https://github.com/cloudpipe/cloudpickle/pull/32)
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18282: [SPARK-21070][PYSPARK] Upgrade cloudpickle
Posted by rgbkrk <gi...@git.apache.org>.
Github user rgbkrk commented on the issue:
https://github.com/apache/spark/pull/18282
Note: I don't think I'll have time to finish this PR for a few weeks so this is freely available for someone else to take on.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18282: [SPARK-21070][PYSPARK] Upgrade cloudpickle
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18282
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18282: [SPARK-21070][PYSPARK] Upgrade cloudpickle
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18282
@rgbkrk, Do you maybe know if there critical issues that might block the Spark's release, and if it is easy to move to taking an explicit (pinned) dependency on cloudpickle (rather than keeping Spark's local copy of cloudpickle here)?
I remember @holdenk raised a similar issue before.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18282: [SPARK-21070][PYSPARK] Upgrade cloudpickle
Posted by rgbkrk <gi...@git.apache.org>.
Github user rgbkrk closed the pull request at:
https://github.com/apache/spark/pull/18282
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18282: [SPARK-21070][PYSPARK] Upgrade cloudpickle
Posted by rgbkrk <gi...@git.apache.org>.
Github user rgbkrk commented on the issue:
https://github.com/apache/spark/pull/18282
Works for me, sorry I don't have cycles at the moment.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18282: [SPARK-21070][PYSPARK] Upgrade cloudpickle
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18282
Can one of the admins verify this patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18282: [SPARK-21070][PYSPARK] Upgrade cloudpickle
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18282
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78102/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18282: [SPARK-21070][PYSPARK] Upgrade cloudpickle
Posted by holdenk <gi...@git.apache.org>.
Github user holdenk commented on the issue:
https://github.com/apache/spark/pull/18282
Jenkins, ok to test. Jenkins add to whitelist.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18282: [SPARK-21070][PYSPARK] Upgrade cloudpickle
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18282
**[Test build #78102 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78102/testReport)** for PR 18282 at commit [`b84222b`](https://github.com/apache/spark/commit/b84222b6ea660ce5ec1aedfee50e297b436eb824).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18282: [SPARK-21070][PYSPARK] Upgrade cloudpickle
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18282
**[Test build #78102 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78102/testReport)** for PR 18282 at commit [`b84222b`](https://github.com/apache/spark/commit/b84222b6ea660ce5ec1aedfee50e297b436eb824).
* This patch **fails PySpark unit tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `class _empty_cell_value(object):`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18282: [SPARK-21070][PYSPARK] Upgrade cloudpickle
Posted by llllllllll <gi...@git.apache.org>.
Github user llllllllll commented on the issue:
https://github.com/apache/spark/pull/18282
I am not sure if this affects pyspark but any function object serialized with 0.2.2 or earlier cannot be deserialized with 0.3.0 or greater. I haven't investigated making this compatible because I am not sure if it is needed but I wanted to make sure you are aware.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18282: [SPARK-21070][PYSPARK] Upgrade cloudpickle
Posted by rgbkrk <gi...@git.apache.org>.
Github user rgbkrk commented on the issue:
https://github.com/apache/spark/pull/18282
This fails one test, looking into it now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18282: [SPARK-21070][PYSPARK] Upgrade cloudpickle
Posted by holdenk <gi...@git.apache.org>.
Github user holdenk commented on the issue:
https://github.com/apache/spark/pull/18282
@HyukjinKwon thanks for bringing that up :) We had a PR to consider using py4j as a dependency rather than a copied zip file, @rxin was against the idea. For now I think simply upgrading cloudpickle as is makes sense, and we can revisit the dependency management if the support burden of this approach becomes difficult (or as we start taking on additional Python libraries).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18282: [SPARK-21070][PYSPARK] Upgrade cloudpickle
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18282
@rgbkrk Shall we close this and reopen when it's ready?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18282: [SPARK-21070][PYSPARK] Upgrade cloudpickle
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18282
Yea, my only worry is, I think we already have some customized codes in Spark's copy and wonder if it is difficult to resolve the conflicts here (for me, IIRC, it took me a while to check out each conflict before). I would go +1 if it is easy for those changes. I will take a look at my best to help review.
cc @davies too.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18282: [SPARK-21070][PYSPARK] Upgrade cloudpickle
Posted by holdenk <gi...@git.apache.org>.
Github user holdenk commented on the issue:
https://github.com/apache/spark/pull/18282
Thanks for that point @llllllllll , since cloudpickle isn't used for any data which is persisted to be used between different versions of Spark we don't need the format to be backwards compatible.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org