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

[GitHub] spark pull request: Update FsHistoryProvider.scala

GitHub user 397090770 opened a pull request:

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

    Update FsHistoryProvider.scala

    Handle AccessControlException for no permission dirs

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

    $ git pull https://github.com/397090770/spark master

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

    https://github.com/apache/spark/pull/2546.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 #2546
    
----
commit 45a8ab32759cdb77e27a3e5f7441bc0528f06b8b
Author: yangping.wu <wy...@163.com>
Date:   2014-09-26T11:29:37Z

    Update FsHistoryProvider.scala
    
    Handle AccessControlException for no permission dirs

----


---
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: Update FsHistoryProvider.scala

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

    https://github.com/apache/spark/pull/2546#issuecomment-57396724
  
    Hey @397090770 could you add the JIRA to the title?


---
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: Update FsHistoryProvider.scala

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

    https://github.com/apache/spark/pull/2546#issuecomment-56950436
  
    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: Update FsHistoryProvider.scala

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

    https://github.com/apache/spark/pull/2546#discussion_r18089631
  
    --- Diff: core/src/main/scala/org/apache/spark/deploy/history/FsHistoryProvider.scala ---
    @@ -153,13 +153,18 @@ private[history] class FsHistoryProvider(conf: SparkConf) extends ApplicationHis
           // later than the last known log directory will be loaded.
           var newLastModifiedTime = lastModifiedTime
           val logInfos = logDirs
    -        .filter { dir =>
    -          if (fs.isFile(new Path(dir.getPath(), EventLoggingListener.APPLICATION_COMPLETE))) {
    -            val modTime = getModificationTime(dir)
    -            newLastModifiedTime = math.max(newLastModifiedTime, modTime)
    -            modTime > lastModifiedTime
    -          } else {
    -            false
    +          .filter { dir =>
    +          try {
    --- End diff --
    
    Can you explain why you want to make this change? you didn't include any info in the title or description, nor was there a linked JIRA. The indentation is wrong, and you're catching `Exception`, which is likely too broad. 


---
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: Update FsHistoryProvider.scala

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

    https://github.com/apache/spark/pull/2546#issuecomment-66399032
  
    @397090770 given that a lot of changes have gone in since this was opened, I would recommend that we close this issue for now until we describe the issue in a JIRA.


---
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: Update FsHistoryProvider.scala

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

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


---
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: Update FsHistoryProvider.scala

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

    https://github.com/apache/spark/pull/2546#issuecomment-57396897
  
    Also, could you add a more descriptive PR title? That's the git commit summary, so it should at least describe what the fix is about.


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