You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by Liuchang0812 <gi...@git.apache.org> on 2015/02/14 15:33:01 UTC

[GitHub] spark pull request: [Core]fix format str and log info

GitHub user Liuchang0812 opened a pull request:

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

    [Core]fix format str and log info

    stageId's type is Int.

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

    $ git pull https://github.com/Liuchang0812/spark fix-format-in-scheduler

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

    https://github.com/apache/spark/pull/4606.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 #4606
    
----
commit cec726fac8691e91e7a00754456dca7c3628daf7
Author: liuchang0812 <li...@gmail.com>
Date:   2015-02-14T14:30:37Z

    fix format str and log info

----


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

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [Core]fix format str and log info

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

    https://github.com/apache/spark/pull/4606#discussion_r24714890
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/TaskSchedulerImpl.scala ---
    @@ -200,7 +200,7 @@ private[spark] class TaskSchedulerImpl(
             val execId = taskIdToExecutorId(tid)
             backend.killTask(tid, execId, interruptThread)
           }
    -      tsm.abort("Stage %s cancelled".format(stageId))
    +      tsm.abort("Stage %d was cancelled".format(stageId))
    --- End diff --
    
    Although I'd like to standardize too, I think it's not worth it as I've come to appreciate how annoying merge conflicts are. This just doesn't buy anything.


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

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [Core]fix format str and log info

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

    https://github.com/apache/spark/pull/4606#discussion_r24714714
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/TaskSchedulerImpl.scala ---
    @@ -200,7 +200,7 @@ private[spark] class TaskSchedulerImpl(
             val execId = taskIdToExecutorId(tid)
             backend.killTask(tid, execId, interruptThread)
           }
    -      tsm.abort("Stage %s cancelled".format(stageId))
    +      tsm.abort("Stage %d was cancelled".format(stageId))
    --- End diff --
    
    Better still, change this to use string interpolation. But, printing an int as an int value, and print its toString as a string, gives the same result. This doesn't do anything so I don't think I'd bother with 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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [Core]fix format str and log info

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

    https://github.com/apache/spark/pull/4606#discussion_r24717123
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/TaskSchedulerImpl.scala ---
    @@ -200,7 +200,7 @@ private[spark] class TaskSchedulerImpl(
             val execId = taskIdToExecutorId(tid)
             backend.killTask(tid, execId, interruptThread)
           }
    -      tsm.abort("Stage %s cancelled".format(stageId))
    +      tsm.abort("Stage %d was cancelled".format(stageId))
    --- End diff --
    
    IMHO yes let's not bother with this sort of thing. It's a tiny change but doesn't improve anything and actually makes things a tiny bit more inconsistent.


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

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [Core]fix format str and log info

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

    https://github.com/apache/spark/pull/4606#issuecomment-74377409
  
    Can one of the admins verify this patch?


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

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [Core]fix format str and log info

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

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


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

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [Core]fix format str and log info

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

    https://github.com/apache/spark/pull/4606#discussion_r24714955
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/TaskSchedulerImpl.scala ---
    @@ -200,7 +200,7 @@ private[spark] class TaskSchedulerImpl(
             val execId = taskIdToExecutorId(tid)
             backend.killTask(tid, execId, interruptThread)
           }
    -      tsm.abort("Stage %s cancelled".format(stageId))
    +      tsm.abort("Stage %d was cancelled".format(stageId))
    --- End diff --
    
    understand :smile: . close it now?
     



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

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [Core]fix format str and log info

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

    https://github.com/apache/spark/pull/4606#discussion_r24714872
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/TaskSchedulerImpl.scala ---
    @@ -200,7 +200,7 @@ private[spark] class TaskSchedulerImpl(
             val execId = taskIdToExecutorId(tid)
             backend.killTask(tid, execId, interruptThread)
           }
    -      tsm.abort("Stage %s cancelled".format(stageId))
    +      tsm.abort("Stage %d was cancelled".format(stageId))
    --- End diff --
    
    Yes, they have same result, but i think we should standard this. I would like change all of this to string interpolation if necessary.
    thanks for your comment :)


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

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org