Skip to content

Conversation

@davies
Copy link
Contributor

@davies davies commented Apr 5, 2016

What changes were proposed in this pull request?

Currently, Broaccast.unpersist() will remove the file of broadcast, which should be the behavior of destroy().

This PR added destroy() for Broadcast in Python, to match the sematics in Scala.

How was this patch tested?

Added regression tests.

@davies
Copy link
Contributor Author

davies commented Apr 5, 2016

cc @yhuai

@SparkQA
Copy link

SparkQA commented Apr 6, 2016

Test build #55053 has finished for PR 12189 at commit cdee210.

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

@holdenk
Copy link
Contributor

holdenk commented Apr 6, 2016

Minor point: would it make sense to also update the PyDoc for unpersist to match the scaladoc while we are fixing its behavior anyways?

@yhuai
Copy link
Contributor

yhuai commented Apr 6, 2016

Seems this PR does not change unpersist? Is the behavior of unpersist correct?

@davies
Copy link
Contributor Author

davies commented Apr 6, 2016

@holdenk Updated.

@yhuai Changed unpersist(), the os.unlink() is moved to destroy()

@SparkQA
Copy link

SparkQA commented Apr 6, 2016

Test build #55123 has finished for PR 12189 at commit e0408f3.

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

@yhuai
Copy link
Contributor

yhuai commented Apr 6, 2016

LGTM. Merging to master. Thanks!

@asfgit asfgit closed this in 90ca184 Apr 6, 2016
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.

4 participants