You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by gatorsmile <gi...@git.apache.org> on 2016/05/22 04:52:42 UTC
[GitHub] spark pull request: [SPARK-15470] [SQL] Unify the Configuration In...
GitHub user gatorsmile opened a pull request:
https://github.com/apache/spark/pull/13247
[SPARK-15470] [SQL] Unify the Configuration Interface in SQLContext
#### What changes were proposed in this pull request?
We introduced `RuntimeConfig` in `SQLContext` in the PR https://github.com/apache/spark/pull/12669. Now, `SQLContext` has both `conf` and `runtimeConf`. `SQLContext` is being replaced by `SparkSession`. Like `SparkSession`, we should not have two configuration interfaces. That means, we should not expose `conf` to external users.
This PR contains three major parts:
1. removed `conf` from `SQLContext`.
2. added the missing functions into `RuntimeConfig`, including two `set` functions and one `clear` function.
3. fixed the test cases in `SparkSessionBuilderSuite.scala`. Without this fix, we are unable to individually run the test cases. All the test cases require `initialSession`.
@rxin @andrewor14 @yhuai @cloud-fan Do you think this PR is valid? Thanks!
#### How was this patch tested?
Existing test cases cover it.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/gatorsmile/spark configNew
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/13247.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 #13247
----
commit f5708f52171ef5ce04eb4358d101d55862cc2294
Author: gatorsmile <ga...@gmail.com>
Date: 2016-05-21T21:25:38Z
initial fix.
commit 0808fa13f04a228377be8f1d17d0aa7da4a47aee
Author: gatorsmile <ga...@gmail.com>
Date: 2016-05-22T03:29:38Z
update the test suites.
commit f443064bfabb9e1055d75b7ee1b33085d72b1a3f
Author: gatorsmile <ga...@gmail.com>
Date: 2016-05-22T04:33:35Z
remove conf from SQLContext
----
---
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-15470] [SQL] Unify the Configuration In...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the pull request:
https://github.com/apache/spark/pull/13247#issuecomment-220816722
I see. Thanks!
---
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-15470] [SQL] Unify the Configuration In...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/13247#issuecomment-221168962
Is the idea to have a list of configs that are marked as mutable and explicitly throw exceptions when users modify them?
I still don't see how that relates to the changes here... Why not just have that list in SQLConf?
---
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-15470] [SQL] Unify the Configuration In...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/13247#issuecomment-221343182
Please close this. I just feel it adds virtually no benefit but requires non-trivial changes.
---
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-15470] [SQL] Unify the Configuration In...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the pull request:
https://github.com/apache/spark/pull/13247#issuecomment-221168724
Previously, @yhuai mentions we need to issue exceptions when users change the static configuration at runtime. See https://github.com/apache/spark/pull/13128#issuecomment-220411852
When trying to do it in a cleaner way, I plan to add these logics into `RuntimeConfig`. However, I found `SQLContext` has two entrances for configuration, one is `conf` and another is `runtimeConf`. Thus, I think we have to remove the duplicate before working on it.
Do you think my concern is OK?
---
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-15470] [SQL] Unify the Configuration In...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/13247#issuecomment-220814526
**[Test build #59090 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59090/consoleFull)** for PR 13247 at commit [`f443064`](https://github.com/apache/spark/commit/f443064bfabb9e1055d75b7ee1b33085d72b1a3f).
* This patch **fails to build**.
* 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 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-15470] [SQL] Unify the Configuration In...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the pull request:
https://github.com/apache/spark/pull/13247#issuecomment-220815210
What's the difference between runtime conf and normal conf?
---
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-15470] [SQL] Unify the Configuration In...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the pull request:
https://github.com/apache/spark/pull/13247#issuecomment-221194357
OK, I can do it by adding a flag. Do you want me to close this PR?
---
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-15470] [SQL] Unify the Configuration In...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/13247#issuecomment-220814528
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59090/
Test FAILed.
---
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-15470] [SQL] Unify the Configuration In...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/13247#issuecomment-220840275
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59108/
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-15470] [SQL] Unify the Configuration In...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/13247#issuecomment-220820105
**[Test build #59091 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59091/consoleFull)** for PR 13247 at commit [`d972e8d`](https://github.com/apache/spark/commit/d972e8d8832c5a04d8ff4cc1ee10293cdbfac977).
* This patch **fails Spark 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 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-15470] [SQL] Unify the Configuration In...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the pull request:
https://github.com/apache/spark/pull/13247#issuecomment-221347186
Sure, let me close it. Thanks!
---
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-15470] [SQL] Unify the Configuration In...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/13247#issuecomment-220816467
I'd like to avoid contaminating RuntimeConfig with private[sql] methods if possible. Those visibility modifiers have no effect in Java.
---
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-15470] [SQL] Unify the Configuration In...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/13247#issuecomment-220819779
**[Test build #59093 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59093/consoleFull)** for PR 13247 at commit [`a8414f4`](https://github.com/apache/spark/commit/a8414f40c2d31f0753d363421d2fae2153ce27db).
---
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-15470] [SQL] Unify the Configuration In...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/13247#issuecomment-220814370
**[Test build #59090 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59090/consoleFull)** for PR 13247 at commit [`f443064`](https://github.com/apache/spark/commit/f443064bfabb9e1055d75b7ee1b33085d72b1a3f).
---
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-15470] [SQL] Unify the Configuration In...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/13247#issuecomment-220820214
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59093/
Test FAILed.
---
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-15470] [SQL] Unify the Configuration In...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/13247#issuecomment-220820109
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59091/
Test FAILed.
---
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-15470] [SQL] Unify the Configuration In...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile closed the pull request at:
https://github.com/apache/spark/pull/13247
---
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-15470] [SQL] Unify the Configuration In...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/13247#issuecomment-220816173
I don't get it. ConfigEntry is internal to Spark. Why are we exposing that?
---
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-15470] [SQL] Unify the Configuration In...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/13247#issuecomment-220840211
**[Test build #59108 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59108/consoleFull)** for PR 13247 at commit [`8a34781`](https://github.com/apache/spark/commit/8a34781bcde12aaa89512eb8fb4dbf028becbfe4).
* This patch passes all 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 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-15470] [SQL] Unify the Configuration In...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/13247#issuecomment-220820212
Merged build finished. Test FAILed.
---
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-15470] [SQL] Unify the Configuration In...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/13247#issuecomment-220835590
**[Test build #59108 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59108/consoleFull)** for PR 13247 at commit [`8a34781`](https://github.com/apache/spark/commit/8a34781bcde12aaa89512eb8fb4dbf028becbfe4).
---
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-15470] [SQL] Unify the Configuration In...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the pull request:
https://github.com/apache/spark/pull/13247#issuecomment-221112165
@rxin I am wondering if what this PR does is OK?
---
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-15470] [SQL] Unify the Configuration In...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the pull request:
https://github.com/apache/spark/pull/13247#issuecomment-221173676
Initially, I did try to do it in that way. The questions bothering me are how to know which changes are made at runtime? which changes are from external users?
After reading the code base, based on my understanding, we will not externalize `SQLConf` after introducing `RumtimeConfig`. The `set` APIs of `SQLConf` will be for internal usage only? Thus, we can do whatever we want, if necessary. For example, to verify the internal behaviors, our test suites are still allowed to change the configuration at runtime. For example, in multiple test cases, like [ddlsuite](https://github.com/apache/spark/blob/5afd927a47aa7ede3039234f2f7262e2247aa2ae/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala#L129), we change `spark.sql.warehouse.dir` at runtime.
Then, I am wondering if we just need to block the changes on the static configuration properties through `RuntimeConfig`? That is the reason why this PR is to remove the `conf` from `SQLContext`.
Feel free to let me know which ways are preferred. Thanks!
---
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-15470] [SQL] Unify the Configuration In...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/13247#issuecomment-220840273
Merged build finished. 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-15470] [SQL] Unify the Configuration In...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/13247#issuecomment-220816493
In this case I guess it is ok though.
---
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-15470] [SQL] Unify the Configuration In...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/13247#issuecomment-220820108
Merged build finished. Test FAILed.
---
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-15470] [SQL] Unify the Configuration In...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the pull request:
https://github.com/apache/spark/pull/13247#issuecomment-220815712
@cloud-fan Based on my understanding, runtime conf (`class RuntimeConfig`) is designed as the public/external interface for users to access the internal conf. If users want to make a change on Config at runtime, they must use `RuntimeConfig`. In the future, we can further enhance it to block external users to change the internal conf? Also easier to manage Hadoop configuration in `RuntimeConfig`?
`SQLConf` will be just an internal implementation of configuration. We do not expect external users to directly access it.
You know, this is just my understanding. : )
---
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-15470] [SQL] Unify the Configuration In...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/13247#issuecomment-220819768
**[Test build #59091 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59091/consoleFull)** for PR 13247 at commit [`d972e8d`](https://github.com/apache/spark/commit/d972e8d8832c5a04d8ff4cc1ee10293cdbfac977).
---
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-15470] [SQL] Unify the Configuration In...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/13247#issuecomment-220814527
Merged build finished. Test FAILed.
---
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-15470] [SQL] Unify the Configuration In...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/13247#issuecomment-221177271
Can we just add a flag to SQLConf to indicate whether SparkSession has been properly initialized?
---
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-15470] [SQL] Unify the Configuration In...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/13247#issuecomment-221343420
One thing that would be a lot more useful is to get rid of SQLContext in our test cases, and use only SparkSession. Then most of the problem in your pr just goes away.
---
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-15470] [SQL] Unify the Configuration In...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/13247#issuecomment-221161280
This looks ok, but can you remind me again what this pull request is actually solving? I feel it's just changing code for the sake of changing code here ...
---
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-15470] [SQL] Unify the Configuration In...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/13247#issuecomment-220820205
**[Test build #59093 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59093/consoleFull)** for PR 13247 at commit [`a8414f4`](https://github.com/apache/spark/commit/a8414f40c2d31f0753d363421d2fae2153ce27db).
* This patch **fails Spark 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 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-15470] [SQL] Unify the Configuration In...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the pull request:
https://github.com/apache/spark/pull/13247#issuecomment-220816386
You are right. Let me add `protected[sql]` before `def set[T](entry: ConfigEntry[T], value: T): Unit`
https://github.com/gatorsmile/spark/blob/d972e8d8832c5a04d8ff4cc1ee10293cdbfac977/sql/core/src/main/scala/org/apache/spark/sql/RuntimeConfig.scala#L67-L69
That will be like what `RuntimeConfig` already has for `get`, which is `protected[sql] def get[T](entry: ConfigEntry[T], default: T): T`
https://github.com/gatorsmile/spark/blob/d972e8d8832c5a04d8ff4cc1ee10293cdbfac977/sql/core/src/main/scala/org/apache/spark/sql/RuntimeConfig.scala#L115-L117
---
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