[jira] [Commented] (SPARK-1527) rootDirs in DiskBlockManagerSuite doesn't get full path from rootDir0, rootDir1

2014-04-24 Thread Niraj Suthar (JIRA)

[ 
https://issues.apache.org/jira/browse/SPARK-1527?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13980185#comment-13980185
 ] 

Niraj Suthar commented on SPARK-1527:
-

Sure Ye Xianjin,

I am more thn happy to do so. after reading the comments I looked at the 
HttpBroadcast.scala and will update it appropriately.

if you guys have any suggestions here..please let me know. 

Thank you,
Niraj

> rootDirs in DiskBlockManagerSuite doesn't get full path from rootDir0, 
> rootDir1
> ---
>
> Key: SPARK-1527
> URL: https://issues.apache.org/jira/browse/SPARK-1527
> Project: Spark
>  Issue Type: Bug
>  Components: Spark Core
>Affects Versions: 0.9.0
>Reporter: Ye Xianjin
>Assignee: Niraj Suthar
>Priority: Minor
>  Labels: starter
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> In core/src/test/scala/org/apache/storage/DiskBlockManagerSuite.scala
>   val rootDir0 = Files.createTempDir()
>   rootDir0.deleteOnExit()
>   val rootDir1 = Files.createTempDir()
>   rootDir1.deleteOnExit()
>   val rootDirs = rootDir0.getName + "," + rootDir1.getName
> rootDir0 and rootDir1 are in system's temporary directory. 
> rootDir0.getName will not get the full path of the directory but the last 
> component of the directory. When passing to DiskBlockManage constructor, the 
> DiskBlockerManger creates directories in pwd not the temporary directory.
> rootDir0.toString will fix this issue.



--
This message was sent by Atlassian JIRA
(v6.2#6252)


[jira] [Commented] (SPARK-1527) rootDirs in DiskBlockManagerSuite doesn't get full path from rootDir0, rootDir1

2014-04-23 Thread Ye Xianjin (JIRA)

[ 
https://issues.apache.org/jira/browse/SPARK-1527?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13979309#comment-13979309
 ] 

Ye Xianjin commented on SPARK-1527:
---

hi, [~nirajsuthar]
This is my pr. https://github.com/apache/spark/pull/436. As [~srowen] said, if 
someone interested in changing the toString()s, please leave a comment.

[~rxin] what do you think?

> rootDirs in DiskBlockManagerSuite doesn't get full path from rootDir0, 
> rootDir1
> ---
>
> Key: SPARK-1527
> URL: https://issues.apache.org/jira/browse/SPARK-1527
> Project: Spark
>  Issue Type: Bug
>  Components: Spark Core
>Affects Versions: 0.9.0
>Reporter: Ye Xianjin
>Assignee: Niraj Suthar
>Priority: Minor
>  Labels: starter
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> In core/src/test/scala/org/apache/storage/DiskBlockManagerSuite.scala
>   val rootDir0 = Files.createTempDir()
>   rootDir0.deleteOnExit()
>   val rootDir1 = Files.createTempDir()
>   rootDir1.deleteOnExit()
>   val rootDirs = rootDir0.getName + "," + rootDir1.getName
> rootDir0 and rootDir1 are in system's temporary directory. 
> rootDir0.getName will not get the full path of the directory but the last 
> component of the directory. When passing to DiskBlockManage constructor, the 
> DiskBlockerManger creates directories in pwd not the temporary directory.
> rootDir0.toString will fix this issue.



--
This message was sent by Atlassian JIRA
(v6.2#6252)


[jira] [Commented] (SPARK-1527) rootDirs in DiskBlockManagerSuite doesn't get full path from rootDir0, rootDir1

2014-04-17 Thread Sean Owen (JIRA)

[ 
https://issues.apache.org/jira/browse/SPARK-1527?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13973148#comment-13973148
 ] 

Sean Owen commented on SPARK-1527:
--

There are a number of other uses of File.getName(), but a quick glance suggests 
all the others are appropriate.

There are a number of other uses of File.toString(), almost all in tests. I 
suspect the Files in question already have absolute paths, and that even 
relative paths happen to work fine in a test since the working dir doesn't 
change. So those could change, but are probably not a concern.

The only one that gave me pause was the use in HttpBroadcast.scala, though I 
suspect it turns out to work fine for similar reasons.

If reviewers are interested in changing the toString()s I'll test and submit a 
PR for that.

> rootDirs in DiskBlockManagerSuite doesn't get full path from rootDir0, 
> rootDir1
> ---
>
> Key: SPARK-1527
> URL: https://issues.apache.org/jira/browse/SPARK-1527
> Project: Spark
>  Issue Type: Bug
>  Components: Spark Core
>Affects Versions: 0.9.0
>Reporter: Ye Xianjin
>Priority: Minor
>  Labels: starter
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> In core/src/test/scala/org/apache/storage/DiskBlockManagerSuite.scala
>   val rootDir0 = Files.createTempDir()
>   rootDir0.deleteOnExit()
>   val rootDir1 = Files.createTempDir()
>   rootDir1.deleteOnExit()
>   val rootDirs = rootDir0.getName + "," + rootDir1.getName
> rootDir0 and rootDir1 are in system's temporary directory. 
> rootDir0.getName will not get the full path of the directory but the last 
> component of the directory. When passing to DiskBlockManage constructor, the 
> DiskBlockerManger creates directories in pwd not the temporary directory.
> rootDir0.toString will fix this issue.



--
This message was sent by Atlassian JIRA
(v6.2#6252)


[jira] [Commented] (SPARK-1527) rootDirs in DiskBlockManagerSuite doesn't get full path from rootDir0, rootDir1

2014-04-17 Thread Ye Xianjin (JIRA)

[ 
https://issues.apache.org/jira/browse/SPARK-1527?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13973096#comment-13973096
 ] 

Ye Xianjin commented on SPARK-1527:
---

Yes, of course, sometimes we want absolute path, sometimes we want to transmit 
a relative path. It depends on logic. 
But I think maybe we should review these usages so that we can make sure 
absolute paths or relative paths are used appropriately.

I may have time to review it after I finish another JIRA issue. If you want to 
take it over, please!

Anyway, thanks for your comments and help.


> rootDirs in DiskBlockManagerSuite doesn't get full path from rootDir0, 
> rootDir1
> ---
>
> Key: SPARK-1527
> URL: https://issues.apache.org/jira/browse/SPARK-1527
> Project: Spark
>  Issue Type: Bug
>  Components: Spark Core
>Affects Versions: 0.9.0
>Reporter: Ye Xianjin
>Priority: Minor
>  Labels: starter
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> In core/src/test/scala/org/apache/storage/DiskBlockManagerSuite.scala
>   val rootDir0 = Files.createTempDir()
>   rootDir0.deleteOnExit()
>   val rootDir1 = Files.createTempDir()
>   rootDir1.deleteOnExit()
>   val rootDirs = rootDir0.getName + "," + rootDir1.getName
> rootDir0 and rootDir1 are in system's temporary directory. 
> rootDir0.getName will not get the full path of the directory but the last 
> component of the directory. When passing to DiskBlockManage constructor, the 
> DiskBlockerManger creates directories in pwd not the temporary directory.
> rootDir0.toString will fix this issue.



--
This message was sent by Atlassian JIRA
(v6.2#6252)


[jira] [Commented] (SPARK-1527) rootDirs in DiskBlockManagerSuite doesn't get full path from rootDir0, rootDir1

2014-04-17 Thread Sean Owen (JIRA)

[ 
https://issues.apache.org/jira/browse/SPARK-1527?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13973091#comment-13973091
 ] 

Sean Owen commented on SPARK-1527:
--

If the paths are only used locally, then an absolute path never hurts (except 
to be a bit longer). I assume that since these are references to a temp 
directory that is by definition only valid locally, that absolute path is the 
right thing to use.

In other cases, similar logic may apply. I could imagine in some cases the 
right thing to do is transmit a relative path. 

> rootDirs in DiskBlockManagerSuite doesn't get full path from rootDir0, 
> rootDir1
> ---
>
> Key: SPARK-1527
> URL: https://issues.apache.org/jira/browse/SPARK-1527
> Project: Spark
>  Issue Type: Bug
>  Components: Spark Core
>Affects Versions: 0.9.0
>Reporter: Ye Xianjin
>Priority: Minor
>  Labels: starter
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> In core/src/test/scala/org/apache/storage/DiskBlockManagerSuite.scala
>   val rootDir0 = Files.createTempDir()
>   rootDir0.deleteOnExit()
>   val rootDir1 = Files.createTempDir()
>   rootDir1.deleteOnExit()
>   val rootDirs = rootDir0.getName + "," + rootDir1.getName
> rootDir0 and rootDir1 are in system's temporary directory. 
> rootDir0.getName will not get the full path of the directory but the last 
> component of the directory. When passing to DiskBlockManage constructor, the 
> DiskBlockerManger creates directories in pwd not the temporary directory.
> rootDir0.toString will fix this issue.



--
This message was sent by Atlassian JIRA
(v6.2#6252)


[jira] [Commented] (SPARK-1527) rootDirs in DiskBlockManagerSuite doesn't get full path from rootDir0, rootDir1

2014-04-17 Thread Ye Xianjin (JIRA)

[ 
https://issues.apache.org/jira/browse/SPARK-1527?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13973087#comment-13973087
 ] 

Ye Xianjin commented on SPARK-1527:
---

Yes. You are right. toString() may give relative path. And since it's 
determined by java.io.tmpdir system property. see 
https://code.google.com/p/guava-libraries/source/browse/guava/src/com/google/common/io/Files.java
 line 591. It's possible that the DiskBlockManager will create different 
directories than the original temp dir when java.io.tmpdir is a relative path. 

so use getAbsolutePath since I use this method in my last pr?

But, I saw toString() was called other places! Should we do something about 
that?

> rootDirs in DiskBlockManagerSuite doesn't get full path from rootDir0, 
> rootDir1
> ---
>
> Key: SPARK-1527
> URL: https://issues.apache.org/jira/browse/SPARK-1527
> Project: Spark
>  Issue Type: Bug
>  Components: Spark Core
>Affects Versions: 0.9.0
>Reporter: Ye Xianjin
>Priority: Minor
>  Labels: starter
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> In core/src/test/scala/org/apache/storage/DiskBlockManagerSuite.scala
>   val rootDir0 = Files.createTempDir()
>   rootDir0.deleteOnExit()
>   val rootDir1 = Files.createTempDir()
>   rootDir1.deleteOnExit()
>   val rootDirs = rootDir0.getName + "," + rootDir1.getName
> rootDir0 and rootDir1 are in system's temporary directory. 
> rootDir0.getName will not get the full path of the directory but the last 
> component of the directory. When passing to DiskBlockManage constructor, the 
> DiskBlockerManger creates directories in pwd not the temporary directory.
> rootDir0.toString will fix this issue.



--
This message was sent by Atlassian JIRA
(v6.2#6252)


[jira] [Commented] (SPARK-1527) rootDirs in DiskBlockManagerSuite doesn't get full path from rootDir0, rootDir1

2014-04-17 Thread Sean Owen (JIRA)

[ 
https://issues.apache.org/jira/browse/SPARK-1527?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13973067#comment-13973067
 ] 

Sean Owen commented on SPARK-1527:
--

{{toString()}} returns {{getPath()}} which may still be relative. 
{{getAbsolutePath()}} is better, but even {{getCanonicalPath()}} may be better 
still.

> rootDirs in DiskBlockManagerSuite doesn't get full path from rootDir0, 
> rootDir1
> ---
>
> Key: SPARK-1527
> URL: https://issues.apache.org/jira/browse/SPARK-1527
> Project: Spark
>  Issue Type: Bug
>  Components: Spark Core
>Affects Versions: 0.9.0
>Reporter: Ye Xianjin
>Priority: Minor
>  Labels: starter
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> In core/src/test/scala/org/apache/storage/DiskBlockManagerSuite.scala
>   val rootDir0 = Files.createTempDir()
>   rootDir0.deleteOnExit()
>   val rootDir1 = Files.createTempDir()
>   rootDir1.deleteOnExit()
>   val rootDirs = rootDir0.getName + "," + rootDir1.getName
> rootDir0 and rootDir1 are in system's temporary directory. 
> rootDir0.getName will not get the full path of the directory but the last 
> component of the directory. When passing to DiskBlockManage constructor, the 
> DiskBlockerManger creates directories in pwd not the temporary directory.
> rootDir0.toString will fix this issue.



--
This message was sent by Atlassian JIRA
(v6.2#6252)