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

[BEAM-1087] Fix inconsistent pickle from dill when dumping the session #1485

Closed

Conversation

sb2nov
Copy link
Contributor

@sb2nov sb2nov commented Dec 1, 2016

Be sure to do all of the following to help us incorporate your contribution
quickly and easily:

  • Make sure the PR title is formatted like:
    [BEAM-<Jira issue #>] Description of pull request
  • Make sure tests pass via mvn clean verify. (Even better, enable
    Travis-CI on your fork and ensure the whole test matrix passes).
  • Replace <Jira issue #> in the title with the actual Jira issue
    number, if there is one.
  • If this contribution is large, please file an Apache
    Individual Contributor License Agreement.

This PR tries to fix GoogleCloudPlatform/DataflowPythonSDK#31 with adding a reduce hook to the Step and Job classes. We also need to modify the dump session so that the pickling is consistent due to uqfoundation/dill#195.

R: @robertwb PTAL

@asfbot
Copy link

asfbot commented Dec 1, 2016

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/5438/
--none--

@asfbot
Copy link

asfbot commented Dec 1, 2016

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/5439/
--none--

@asfbot
Copy link

asfbot commented Dec 2, 2016

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/5466/
--none--

@sb2nov sb2nov force-pushed the BEAM-test-main-session-pickle branch 2 times, most recently from 4bb4148 to 54b969f Compare December 2, 2016 23:06
@asfbot
Copy link

asfbot commented Dec 3, 2016

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/5487/
--none--

@asfbot
Copy link

asfbot commented Dec 3, 2016

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/5489/
--none--

@sb2nov sb2nov force-pushed the BEAM-test-main-session-pickle branch from 54b969f to 3658793 Compare December 5, 2016 19:22
@sb2nov sb2nov changed the title WIP - Beam test main session pickle [BEAM-1087] Fix inconsistent pickle from dill when dumping the session Dec 5, 2016
@asfbot
Copy link

asfbot commented Dec 5, 2016

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/5525/
--none--

@sb2nov
Copy link
Contributor Author

sb2nov commented Dec 5, 2016

R: @robertwb PTAL

@robertwb
Copy link
Contributor

robertwb commented Dec 6, 2016

Ideally we shouldn't be pickling any live objects from apiclient.py... can this be avoided?

@sb2nov
Copy link
Contributor Author

sb2nov commented Dec 6, 2016

There is a dangerous looking solution in uqfoundation/dill#66 which could be useful in ignoring the objects in apiclient but I think that might add complexity compared to the adding reduce methods to the two classes. Thoughts ?

@robertwb
Copy link
Contributor

robertwb commented Dec 6, 2016

I think this is good for now, and in general it'd be better to have more robust session (or at least main module) pickling. LGTM

asfgit pushed a commit that referenced this pull request Dec 6, 2016
@sb2nov
Copy link
Contributor Author

sb2nov commented Dec 6, 2016

Thanks. Yeah, having a more robust pickling story would be great.

@sb2nov sb2nov closed this Dec 6, 2016
@sb2nov sb2nov deleted the BEAM-test-main-session-pickle branch December 6, 2016 18:22
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.

None yet

3 participants