Skip to content

SPARK-10701 Expose SparkContext#stopped flag with @DeveloperApi#8822

Closed
tedyu wants to merge 1 commit intoapache:masterfrom
tedyu:master
Closed

SPARK-10701 Expose SparkContext#stopped flag with @DeveloperApi#8822
tedyu wants to merge 1 commit intoapache:masterfrom
tedyu:master

Conversation

@tedyu
Copy link
Contributor

@tedyu tedyu commented Sep 18, 2015

See this thread: http://search-hadoop.com/m/q3RTtqvncy17sSTx1

We should expose this flag to developers

@andrewor14

@tedyu tedyu changed the title Expose SparkContext#stopped flag with @DeveloperApi SPARK-10701 Expose SparkContext#stopped flag with @DeveloperApi Sep 18, 2015
Copy link
Contributor

Choose a reason for hiding this comment

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

can we expose a slightly nicer API? Right now this is an atomic boolean and the user has to do something like
sc.stopped.get which is super clunky. It would be better if they can do sc.isStopped or something

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, Andrew.

@andrewor14
Copy link
Contributor

@srowen @JoshRosen do you guys think this is a good idea?

@rxin
Copy link
Contributor

rxin commented Sep 18, 2015

-1

I don't think we should just expose an atomic variable directly. Apps can accidentally set it to false.

@andrewor14
Copy link
Contributor

@rxin yes that was my comment, but do you think we should add a getter for it?

@srowen
Copy link
Member

srowen commented Sep 18, 2015

This is a duplicate. Let's move convo to #8749 Yes, definitely not OK to expose the mutable object.

@andrewor14
Copy link
Contributor

OK let's close this PR then.

@tedyu tedyu closed this Sep 18, 2015
@SparkQA
Copy link

SparkQA commented Sep 18, 2015

Test build #42673 has finished for PR 8822 at commit 007c6d6.

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

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.

5 participants