You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by andrewor14 <gi...@git.apache.org> on 2014/05/05 20:19:01 UTC

[GitHub] spark pull request: Expose SparkListeners and relevant classes as ...

GitHub user andrewor14 opened a pull request:

    https://github.com/apache/spark/pull/648

    Expose SparkListeners and relevant classes as DeveloperApi

    Hopefully this can go into 1.0, as a few people on the user list have asked for this.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/andrewor14/spark expose-listeners

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/648.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 #648
    
----
commit 350d643759385c0ae7dfa83d2c7fdf44f8d38de1
Author: Andrew Or <an...@gmail.com>
Date:   2014-05-05T18:15:43Z

    Expose SparkListeners and relevant classes as DeveloperApi

----


---
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.
---

[GitHub] spark pull request: Expose SparkListeners and relevant classes as ...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/648#discussion_r12302370
  
    --- Diff: core/src/main/scala/org/apache/spark/storage/BlockId.scala ---
    @@ -19,14 +19,18 @@ package org.apache.spark.storage
     
     import java.util.UUID
     
    +import org.apache.spark.annotation.DeveloperApi
    +
     /**
    + * :: DeveloperApi ::
      * Identifies a particular Block of data, usually associated with a single file.
      * A Block can be uniquely identified by its filename, but each type of Block has a different
      * set of keys which produce its unique name.
      *
      * If your BlockId should be serializable, be sure to add it to the BlockId.apply() method.
      */
    -private[spark] sealed abstract class BlockId {
    +@DeveloperApi
    +sealed abstract class BlockId {
    --- End diff --
    
    It would be a fairly big change, since we technically don't need the BlockId classes anymore, as the strings are supposedly globally unique. I'm inclined to just leave it.


---
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.
---

[GitHub] spark pull request: Expose SparkListeners and relevant classes as ...

Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:

    https://github.com/apache/spark/pull/648#issuecomment-42259578
  
    Thanks, I merged this.


---
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.
---

[GitHub] spark pull request: Expose SparkListeners and relevant classes as ...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/648


---
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.
---

[GitHub] spark pull request: Expose SparkListeners and relevant classes as ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/648#issuecomment-42225582
  
    All automated tests passed.
    Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/14670/


---
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.
---

[GitHub] spark pull request: Expose SparkListeners and relevant classes as ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/648#issuecomment-42225578
  
    Merged build finished. All automated tests passed.


---
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.
---

[GitHub] spark pull request: Expose SparkListeners and relevant classes as ...

Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on a diff in the pull request:

    https://github.com/apache/spark/pull/648#discussion_r12291537
  
    --- Diff: core/src/main/scala/org/apache/spark/storage/BlockId.scala ---
    @@ -19,14 +19,18 @@ package org.apache.spark.storage
     
     import java.util.UUID
     
    +import org.apache.spark.annotation.DeveloperApi
    +
     /**
    + * :: DeveloperApi ::
      * Identifies a particular Block of data, usually associated with a single file.
      * A Block can be uniquely identified by its filename, but each type of Block has a different
      * set of keys which produce its unique name.
      *
      * If your BlockId should be serializable, be sure to add it to the BlockId.apply() method.
      */
    -private[spark] sealed abstract class BlockId {
    +@DeveloperApi
    +sealed abstract class BlockId {
    --- End diff --
    
    Can we just expose BlockId's as strings instead of revealing this class?


---
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.
---

[GitHub] spark pull request: Expose SparkListeners and relevant classes as ...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/648#discussion_r12294450
  
    --- Diff: core/src/main/scala/org/apache/spark/storage/BlockId.scala ---
    @@ -19,14 +19,18 @@ package org.apache.spark.storage
     
     import java.util.UUID
     
    +import org.apache.spark.annotation.DeveloperApi
    +
     /**
    + * :: DeveloperApi ::
      * Identifies a particular Block of data, usually associated with a single file.
      * A Block can be uniquely identified by its filename, but each type of Block has a different
      * set of keys which produce its unique name.
      *
      * If your BlockId should be serializable, be sure to add it to the BlockId.apply() method.
      */
    -private[spark] sealed abstract class BlockId {
    +@DeveloperApi
    +sealed abstract class BlockId {
    --- End diff --
    
    Do you mean exposing `def name` only? 


---
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.
---

[GitHub] spark pull request: Expose SparkListeners and relevant classes as ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/648#issuecomment-42219881
  
     Merged build triggered. 


---
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.
---

[GitHub] spark pull request: Expose SparkListeners and relevant classes as ...

Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on a diff in the pull request:

    https://github.com/apache/spark/pull/648#discussion_r12301776
  
    --- Diff: core/src/main/scala/org/apache/spark/storage/BlockId.scala ---
    @@ -19,14 +19,18 @@ package org.apache.spark.storage
     
     import java.util.UUID
     
    +import org.apache.spark.annotation.DeveloperApi
    +
     /**
    + * :: DeveloperApi ::
      * Identifies a particular Block of data, usually associated with a single file.
      * A Block can be uniquely identified by its filename, but each type of Block has a different
      * set of keys which produce its unique name.
      *
      * If your BlockId should be serializable, be sure to add it to the BlockId.apply() method.
      */
    -private[spark] sealed abstract class BlockId {
    +@DeveloperApi
    +sealed abstract class BlockId {
    --- End diff --
    
    I just mean we should pass these around as String's in the events. Would that be a big change?


---
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.
---

[GitHub] spark pull request: Expose SparkListeners and relevant classes as ...

Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on a diff in the pull request:

    https://github.com/apache/spark/pull/648#discussion_r12301802
  
    --- Diff: core/src/main/scala/org/apache/spark/storage/BlockId.scala ---
    @@ -19,14 +19,18 @@ package org.apache.spark.storage
     
     import java.util.UUID
     
    +import org.apache.spark.annotation.DeveloperApi
    +
     /**
    + * :: DeveloperApi ::
      * Identifies a particular Block of data, usually associated with a single file.
      * A Block can be uniquely identified by its filename, but each type of Block has a different
      * set of keys which produce its unique name.
      *
      * If your BlockId should be serializable, be sure to add it to the BlockId.apply() method.
      */
    -private[spark] sealed abstract class BlockId {
    +@DeveloperApi
    +sealed abstract class BlockId {
    --- End diff --
    
    Maybe it makes sense to give users some more semantics about this... it might be good to leave it as is


---
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.
---

[GitHub] spark pull request: Expose SparkListeners and relevant classes as ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/648#issuecomment-42219901
  
    Merged build started. 


---
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.
---