You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by srowen <gi...@git.apache.org> on 2015/04/05 11:09:56 UTC
[GitHub] spark pull request: SPARK-6630 [CORE] SparkConf.setIfMissing shoul...
GitHub user srowen opened a pull request:
https://github.com/apache/spark/pull/5365
SPARK-6630 [CORE] SparkConf.setIfMissing should only evaluate the assigned value if indeed missing
Only evaluate value argument to `setIfMissing` if property value is actually missing and will be set.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/srowen/spark SPARK-6630
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/5365.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 #5365
----
commit 88be73f949df047e3457458c822c97297a09aec0
Author: Sean Owen <so...@cloudera.com>
Date: 2015-04-05T09:00:39Z
Only evaluate value argument to `setIfMissing` if property value is actually missing and will be set.
----
---
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: SPARK-6630 [CORE] SparkConf.setIfMissing shoul...
Posted by srowen <gi...@git.apache.org>.
Github user srowen closed the pull request at:
https://github.com/apache/spark/pull/5365
---
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: SPARK-6630 [CORE] SparkConf.setIfMissing shoul...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/5365#issuecomment-89830292
I looked into the Jenkins log, since the Github auth issue prevented it from posting results besides the unit tests. I was surprised that it did not report binary incompatibility. When I look at the bytecode before and after, it does clearly seem to change the signature:
```
public setIfMissing(Ljava/lang/String;Ljava/lang/String;)Lorg/apache/spark/SparkConf;
```
```
public setIfMissing(Ljava/lang/String;Lscala/Function0;)Lorg/apache/spark/SparkConf;
```
So, I think we can't do this. An overload defeats the purpose, right?
---
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: SPARK-6630 [CORE] SparkConf.setIfMissing shoul...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/5365#issuecomment-89751479
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/29722/
Test 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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-6630 [CORE] SparkConf.setIfMissing shoul...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/5365#issuecomment-89738660
I have a feeling this won't be binary compatible but wanted to see.
---
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