[GitHub] spark pull request: SPARK-3177 (on Master Branch)

2014-09-17 Thread tgravescs 
Github user tgravescs commented on the pull request:

https://github.com/apache/spark/pull/2204#issuecomment-55910777
  
looks good. I tested it on hadoop 0.23 and it works also.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: SPARK-3177 (on Master Branch)

2014-09-17 Thread asfgit 
Github user asfgit closed the pull request at:

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


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: SPARK-3177 (on Master Branch)

2014-09-16 Thread andrewor14 
Github user andrewor14 commented on the pull request:

https://github.com/apache/spark/pull/2204#issuecomment-55784185
  
@tgravescs


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: SPARK-3177 (on Master Branch)

2014-09-14 Thread chesterxgchen 
Github user chesterxgchen commented on the pull request:

https://github.com/apache/spark/pull/2204#issuecomment-55545571
  
Reformatted based on Andrew's comment #2204


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: SPARK-3177 (on Master Branch)

2014-09-14 Thread SparkQA 
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/2204#issuecomment-55545608
  
  [QA tests have 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/20314/consoleFull)
 for   PR 2204 at commit 
[`e72a6ea`](https://github.com/apache/spark/commit/e72a6eab54d2b93d09a18b33670f398411cd17a1).
 * This patch merges cleanly.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: SPARK-3177 (on Master Branch)

2014-09-14 Thread SparkQA 
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/2204#issuecomment-55547575
  
  [QA tests have 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/20314/consoleFull)
 for   PR 2204 at commit 
[`e72a6ea`](https://github.com/apache/spark/commit/e72a6eab54d2b93d09a18b33670f398411cd17a1).
 * This patch **passes** unit tests.
 * This patch merges cleanly.
 * This patch adds no public classes.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: SPARK-3177 (on Master Branch)

2014-09-09 Thread andrewor14 
Github user andrewor14 commented on the pull request:

https://github.com/apache/spark/pull/2204#issuecomment-54996816
  
ok to test


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: SPARK-3177 (on Master Branch)

2014-09-09 Thread SparkQA 
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/2204#issuecomment-55000338
  
  [QA tests have 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/20046/consoleFull)
 for   PR 2204 at commit 
[`77be5d0`](https://github.com/apache/spark/commit/77be5d0640f8a61e525e6ef8b59fc202b14e7798).
 * This patch merges cleanly.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: SPARK-3177 (on Master Branch)

2014-09-09 Thread SparkQA 
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/2204#issuecomment-55007803
  
  [QA tests have 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/20046/consoleFull)
 for   PR 2204 at commit 
[`77be5d0`](https://github.com/apache/spark/commit/77be5d0640f8a61e525e6ef8b59fc202b14e7798).
 * This patch **fails** unit tests.
 * This patch merges cleanly.
 * This patch adds no public classes.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: SPARK-3177 (on Master Branch)

2014-09-09 Thread chesterxgchen 
Github user chesterxgchen commented on the pull request:

https://github.com/apache/spark/pull/2204#issuecomment-55013602
  
The unit test failed *sql/test:test*
I run it locally, with the same configuration, the test passed.

the changes is for yarn unit test, it doesn't affect the *sql/test.*


On Tue, Sep 9, 2014 at 10:44 AM, Apache Spark QA notificati...@github.com
wrote:

 QA tests have finished
 
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/20046/consoleFull
 for PR 2204 at commit 77be5d0
 
https://github.com/apache/spark/commit/77be5d0640f8a61e525e6ef8b59fc202b14e7798
 .

- This patch *fails* unit tests.
- This patch merges cleanly.
- This patch adds no public classes.

 —
 Reply to this email directly or view it on GitHub
 https://github.com/apache/spark/pull/2204#issuecomment-55007803.



---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: SPARK-3177 (on Master Branch)

2014-09-09 Thread andrewor14 
Github user andrewor14 commented on a diff in the pull request:

https://github.com/apache/spark/pull/2204#discussion_r17331761
  
--- Diff: 
yarn/common/src/test/scala/org/apache/spark/deploy/yarn/ClientBaseSuite.scala 
---
@@ -232,6 +233,15 @@ class ClientBaseSuite extends FunSuite with Matchers {
   def getFieldValue[A, B](clazz: Class[_], field: String, defaults: = 
B)(mapTo: A = B): B =
 
Try(clazz.getField(field)).map(_.get(null).asInstanceOf[A]).toOption.map(mapTo).getOrElse(defaults)
 
+  def getFieldValue2[A: ClassTag, A1: ClassTag, B](clazz: Class[_], field: 
String,
+  defaults: = B)
+  (mapTo:  A = B)(mapTo1: A1 = B) : B =
--- End diff --

Can you structure this as follows:

```
def getFieldValue[...](
arg1: String,
arg2: String,
...)(mapTo: A = B)(mapTo1: A1 = B): B = {
  Try(...)
}
```


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: SPARK-3177 (on Master Branch)

2014-09-09 Thread andrewor14 
Github user andrewor14 commented on a diff in the pull request:

https://github.com/apache/spark/pull/2204#discussion_r17331783
  
--- Diff: 
yarn/common/src/test/scala/org/apache/spark/deploy/yarn/ClientBaseSuite.scala 
---
@@ -232,6 +233,15 @@ class ClientBaseSuite extends FunSuite with Matchers {
   def getFieldValue[A, B](clazz: Class[_], field: String, defaults: = 
B)(mapTo: A = B): B =
 
Try(clazz.getField(field)).map(_.get(null).asInstanceOf[A]).toOption.map(mapTo).getOrElse(defaults)
 
+  def getFieldValue2[A: ClassTag, A1: ClassTag, B](clazz: Class[_], field: 
String,
+  defaults: = B)
+  (mapTo:  A = B)(mapTo1: A1 = B) : B =
--- End diff --

(the style in the rest of this file is not a great example to follow!)


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: SPARK-3177 (on Master Branch)

2014-09-05 Thread SparkQA 
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/2204#issuecomment-54694372
  
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: SPARK-3177 (on Master Branch)

2014-09-03 Thread ScrapCodes 
Github user ScrapCodes commented on a diff in the pull request:

https://github.com/apache/spark/pull/2204#discussion_r17032605
  
--- Diff: 
yarn/common/src/test/scala/org/apache/spark/deploy/yarn/ClientBaseSuite.scala 
---
@@ -232,6 +233,15 @@ class ClientBaseSuite extends FunSuite with Matchers {
   def getFieldValue[A, B](clazz: Class[_], field: String, defaults: = 
B)(mapTo: A = B): B =
 
Try(clazz.getField(field)).map(_.get(null).asInstanceOf[A]).toOption.map(mapTo).getOrElse(defaults)
 
+  def getFieldValue2[A: ClassTag, A1: ClassTag, B](clazz: Class[_], field: 
String,
--- End diff --

This is not true.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: SPARK-3177 (on Master Branch)

2014-09-03 Thread chesterxgchen 
Github user chesterxgchen commented on a diff in the pull request:

https://github.com/apache/spark/pull/2204#discussion_r17048232
  
--- Diff: 
yarn/common/src/test/scala/org/apache/spark/deploy/yarn/ClientBaseSuite.scala 
---
@@ -232,6 +233,15 @@ class ClientBaseSuite extends FunSuite with Matchers {
   def getFieldValue[A, B](clazz: Class[_], field: String, defaults: = 
B)(mapTo: A = B): B =
 
Try(clazz.getField(field)).map(_.get(null).asInstanceOf[A]).toOption.map(mapTo).getOrElse(defaults)
 
+  def getFieldValue2[A: ClassTag, A1: ClassTag, B](clazz: Class[_], field: 
String,
--- End diff --

Can you elaborate this ?  What's not true ?

Remove the classTag and run the unit test and see yourself. 

Sent from my iPhone

 On Sep 2, 2014, at 11:00 PM, Prashant Sharma notificati...@github.com 
wrote:
 
 In 
yarn/common/src/test/scala/org/apache/spark/deploy/yarn/ClientBaseSuite.scala:
 
  @@ -232,6 +233,15 @@ class ClientBaseSuite extends FunSuite with 
Matchers {
 def getFieldValue[A, B](clazz: Class[_], field: String, defaults: = 
B)(mapTo: A = B): B =
   
Try(clazz.getField(field)).map(_.get(null).asInstanceOf[A]).toOption.map(mapTo).getOrElse(defaults)
   
  +  def getFieldValue2[A: ClassTag, A1: ClassTag, B](clazz: Class[_], 
field: String,
 This is not true.
 
 —
 Reply to this email directly or view it on GitHub.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: SPARK-3177 (on Master Branch)

2014-09-01 Thread ScrapCodes 
Github user ScrapCodes commented on a diff in the pull request:

https://github.com/apache/spark/pull/2204#discussion_r16945967
  
--- Diff: 
yarn/common/src/test/scala/org/apache/spark/deploy/yarn/ClientBaseSuite.scala 
---
@@ -232,6 +233,15 @@ class ClientBaseSuite extends FunSuite with Matchers {
   def getFieldValue[A, B](clazz: Class[_], field: String, defaults: = 
B)(mapTo: A = B): B =
 
Try(clazz.getField(field)).map(_.get(null).asInstanceOf[A]).toOption.map(mapTo).getOrElse(defaults)
 
+  def getFieldValue2[A: ClassTag, A1: ClassTag, B](clazz: Class[_], field: 
String,
--- End diff --

Why do you need `ClassTag` ?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: SPARK-3177 (on Master Branch)

2014-09-01 Thread chesterxgchen 
Github user chesterxgchen commented on a diff in the pull request:

https://github.com/apache/spark/pull/2204#discussion_r16964789
  
--- Diff: 
yarn/common/src/test/scala/org/apache/spark/deploy/yarn/ClientBaseSuite.scala 
---
@@ -232,6 +233,15 @@ class ClientBaseSuite extends FunSuite with Matchers {
   def getFieldValue[A, B](clazz: Class[_], field: String, defaults: = 
B)(mapTo: A = B): B =
 
Try(clazz.getField(field)).map(_.get(null).asInstanceOf[A]).toOption.map(mapTo).getOrElse(defaults)
 
+  def getFieldValue2[A: ClassTag, A1: ClassTag, B](clazz: Class[_], field: 
String,
--- End diff --

without ClassTag, you will get the following warnings:
 
 abstract type pattern A is unchecked since it is eliminated by erasure
 abstract type pattern A1 is unchecked since it is eliminated by erasure

so that 
case v: A = mapTo(v)
case v1: A1 = mapTo1(v1)

will not match

with ClassTag, the runtime type are returned and above match will work as 
expected







---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: SPARK-3177 (on Master Branch)

2014-08-29 Thread chesterxgchen 
GitHub user chesterxgchen opened a pull request:

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

SPARK-3177 (on Master Branch)

The JIRA and PR was original created for branch-1.1, and move to master 
branch now. 
Chester

The Issue is due to that yarn-alpha and yarn have different APIs for 
certain class fields. In this particular case,  the ClientBase using reflection 
to to address this issue, and we need to different way to test the ClientBase's 
method.  Original ClientBaseSuite using getFieldValue() method to do this. But 
it doesn't work for yarn-alpha as the API returns an array of String instead of 
just String (which is the case for Yarn-stable API).

 To fix the test, I add a new method

  def getFieldValue2[A: ClassTag, A1: ClassTag, B](clazz: Class[_], field: 
String,
  defaults: = B)
  (mapTo:  A = B)(mapTo1: A1 = B) : B =
Try(clazz.getField(field)).map(_.get(null)).map {
  case v: A = mapTo(v)
  case v1: A1 = mapTo1(v1)
  case _ = defaults
}.toOption.getOrElse(defaults)

to handle the cases where the field type can be either type A or A1. In 
this new method the type A or A1 is pattern matched and corresponding mapTo 
function (mapTo or mapTo1) is used.

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

$ git pull https://github.com/AlpineNow/spark SPARK-3177-master

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

https://github.com/apache/spark/pull/2204.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 #2204


commit 77be5d0640f8a61e525e6ef8b59fc202b14e7798
Author: chesterxgchen ches...@alpinenow.com
Date:   2014-08-29T18:48:51Z

 The Issue is due to that yarn-alpha and yarn have different APIs for 
certain class fields. In this particular case,  the ClientBase using reflection 
to to address this issue, and we need to different way to test the ClientBase's 
method.  Original ClientBaseSuite using getFieldValue() method to do this. But 
it doesn't work for yarn-alpha as the API returns an array of String instead of 
just String (which is the case for Yarn-stable API).

 To fix the test, I add a new method

  def getFieldValue2[A: ClassTag, A1: ClassTag, B](clazz: Class[_], field: 
String,
  defaults: = B)
  (mapTo:  A = B)(mapTo1: A1 = B) : B =
Try(clazz.getField(field)).map(_.get(null)).map {
  case v: A = mapTo(v)
  case v1: A1 = mapTo1(v1)
  case _ = defaults
}.toOption.getOrElse(defaults)

to handle the cases where the field type can be either type A or A1. In 
this new method the type A or A1 is pattern matched and corresponding mapTo 
function (mapTo or mapTo1) is used.




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: SPARK-3177 (on Master Branch)

2014-08-29 Thread AmplabJenkins 
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/2204#issuecomment-53916073
  
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: SPARK-3177 (on Master Branch)

2014-08-29 Thread chesterxgchen 
Github user chesterxgchen commented on the pull request:

https://github.com/apache/spark/pull/2204#issuecomment-53916231
  
I have tested against 2.0.5-alpha, 2.0.6-alpha (yarn-alpha) and 
2.3.0-cdh5.0.0 (yarn-stable) 
all yarn-alpha/test and yarn-stable/test are 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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