Skip to content

Conversation

@davies
Copy link
Contributor

@davies davies commented Jun 20, 2016

What changes were proposed in this pull request?

In the case that we don't know which module a object came from, will call pickle.whichmodule() to go throught all the loaded modules to find the object, which could fail because some modules, for example, six, see https://bitbucket.org/gutworth/six/issues/63/importing-six-breaks-pickling

We should ignore the exception here, use __main__ as the module name (it means we can't find the module).

How was this patch tested?

Manual tested. Can't have a unit test for this.

@SparkQA
Copy link

SparkQA commented Jun 20, 2016

Test build #60877 has finished for PR 13788 at commit 0d3df9a.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@davies
Copy link
Contributor Author

davies commented Jun 23, 2016

cc @JoshRosen

modname = pickle.whichmodule(obj, name)
try:
modname = pickle.whichmodule(obj, name)
except Exception:
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a narrower exception that we can catch (ImportError perhaps)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will run into arbitary code, so anything could happen

@JoshRosen
Copy link
Contributor

Do you mind leaving a comment in the code referencing this issue / maybe linking to that six issue so that it's easier for a reader to figure out what's going on? Otherwise, this looks good to me.

@SparkQA
Copy link

SparkQA commented Jun 24, 2016

Test build #61198 has finished for PR 13788 at commit b823562.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

asfgit pushed a commit that referenced this pull request Jun 24, 2016
## What changes were proposed in this pull request?

In the case that we don't know which module a object came from, will call pickle.whichmodule() to go throught all the loaded modules to find the object, which could fail because some modules, for example, six, see https://bitbucket.org/gutworth/six/issues/63/importing-six-breaks-pickling

We should ignore the exception here, use `__main__` as the module name (it means we can't find the module).

## How was this patch tested?

Manual tested. Can't have a unit test for this.

Author: Davies Liu <[email protected]>

Closes #13788 from davies/whichmodule.

(cherry picked from commit d489354)
Signed-off-by: Davies Liu <[email protected]>
@davies
Copy link
Contributor Author

davies commented Jun 24, 2016

Merged into 1.6, 2.0 and master

@asfgit asfgit closed this in d489354 Jun 24, 2016
asfgit pushed a commit that referenced this pull request Jun 24, 2016
## What changes were proposed in this pull request?

In the case that we don't know which module a object came from, will call pickle.whichmodule() to go throught all the loaded modules to find the object, which could fail because some modules, for example, six, see https://bitbucket.org/gutworth/six/issues/63/importing-six-breaks-pickling

We should ignore the exception here, use `__main__` as the module name (it means we can't find the module).

## How was this patch tested?

Manual tested. Can't have a unit test for this.

Author: Davies Liu <[email protected]>

Closes #13788 from davies/whichmodule.

(cherry picked from commit d489354)
Signed-off-by: Davies Liu <[email protected]>
zzcclp pushed a commit to zzcclp/spark that referenced this pull request Jun 27, 2016
## What changes were proposed in this pull request?

In the case that we don't know which module a object came from, will call pickle.whichmodule() to go throught all the loaded modules to find the object, which could fail because some modules, for example, six, see https://bitbucket.org/gutworth/six/issues/63/importing-six-breaks-pickling

We should ignore the exception here, use `__main__` as the module name (it means we can't find the module).

## How was this patch tested?

Manual tested. Can't have a unit test for this.

Author: Davies Liu <[email protected]>

Closes apache#13788 from davies/whichmodule.

(cherry picked from commit d489354)
Signed-off-by: Davies Liu <[email protected]>
(cherry picked from commit d7223bb)
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.

3 participants