You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by MaxGekk <gi...@git.apache.org> on 2018/07/08 19:36:07 UTC
[GitHub] spark pull request #21730: [SPARK-24761][SQL] Adding of isModifiable() to Ru...
GitHub user MaxGekk opened a pull request:
https://github.com/apache/spark/pull/21730
[SPARK-24761][SQL] Adding of isModifiable() to RuntimeConfig
## What changes were proposed in this pull request?
In the PR, I propose to extend `RuntimeConfig` by new method `isModifiable()` which returns `true` if a config parameter can be modified at runtime (for current session state). For static SQL and core parameters, the method returns `false`.
## How was this patch tested?
Added new test to `RuntimeConfigSuite` for checking Spark core and SQL parameters.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/MaxGekk/spark-1 is-modifiable
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/21730.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 #21730
----
commit c7819e60ff674e929d47b4c7247dea0f85349ac5
Author: Maxim Gekk <ma...@...>
Date: 2018-07-08T18:59:18Z
[SQL] Is a config modifiable at runtime
In the PR, I propose to extend `RuntimeConfig` by new method `isModifiable()` which returns `true` if a config parameter can be modified at runtime (in notebooks). For static SQL and core parameters, the method returns `false`.
commit b90dbf834f7d8a7dc14fcee87968997f80fd52ac
Author: Maxim Gekk <ma...@...>
Date: 2018-07-08T19:32:08Z
Adding ticket number to test's title
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21730: [SPARK-24761][SQL] Adding of isModifiable() to RuntimeCo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21730
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/92721/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21730: [SPARK-24761][SQL] Adding of isModifiable() to Ru...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/21730
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21730: [SPARK-24761][SQL] Adding of isModifiable() to Ru...
Posted by gengliangwang <gi...@git.apache.org>.
Github user gengliangwang commented on a diff in the pull request:
https://github.com/apache/spark/pull/21730#discussion_r201678881
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/RuntimeConfigSuite.scala ---
@@ -54,4 +54,15 @@ class RuntimeConfigSuite extends SparkFunSuite {
conf.get("k1")
}
}
+
+ test("SPARK-24761: is a config parameter modifiable") {
+ val conf = newConf()
+
+ // SQL configs
+ assert(!conf.isModifiable("spark.sql.sources.schemaStringLengthThreshold"))
--- End diff --
nit: add a case for nonexistent sql conf key?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21730: [SPARK-24761][SQL] Adding of isModifiable() to Ru...
Posted by MaxGekk <gi...@git.apache.org>.
Github user MaxGekk commented on a diff in the pull request:
https://github.com/apache/spark/pull/21730#discussion_r200851659
--- Diff: python/pyspark/sql/conf.py ---
@@ -63,6 +63,12 @@ def _checkType(self, obj, identifier):
raise TypeError("expected %s '%s' to be a string (was '%s')" %
(identifier, obj, type(obj).__name__))
+ @ignore_unicode_prefix
+ @since(2.4)
+ def isModifiable(self, key):
+ """Is the configuration property modifiable or not."""
--- End diff --
I am going to change to:
```
Indicates that the configuration property with the given key modifiable in the current session or not.
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21730: [SPARK-24761][SQL] Adding of isModifiable() to RuntimeCo...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21730
**[Test build #92721 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/92721/testReport)** for PR 21730 at commit [`b90dbf8`](https://github.com/apache/spark/commit/b90dbf834f7d8a7dc14fcee87968997f80fd52ac).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21730: [SPARK-24761][SQL] Adding of isModifiable() to RuntimeCo...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21730
**[Test build #92721 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/92721/testReport)** for PR 21730 at commit [`b90dbf8`](https://github.com/apache/spark/commit/b90dbf834f7d8a7dc14fcee87968997f80fd52ac).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21730: [SPARK-24761][SQL] Adding of isModifiable() to RuntimeCo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21730
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21730: [SPARK-24761][SQL] Adding of isModifiable() to RuntimeCo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21730
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21730: [SPARK-24761][SQL] Adding of isModifiable() to Ru...
Posted by ssimeonov <gi...@git.apache.org>.
Github user ssimeonov commented on a diff in the pull request:
https://github.com/apache/spark/pull/21730#discussion_r200851248
--- Diff: python/pyspark/sql/conf.py ---
@@ -63,6 +63,12 @@ def _checkType(self, obj, identifier):
raise TypeError("expected %s '%s' to be a string (was '%s')" %
(identifier, obj, type(obj).__name__))
+ @ignore_unicode_prefix
+ @since(2.4)
+ def isModifiable(self, key):
+ """Is the configuration property modifiable or not."""
--- End diff --
Why not make all doc comments across languages identical?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21730: [SPARK-24761][SQL] Adding of isModifiable() to Ru...
Posted by ssimeonov <gi...@git.apache.org>.
Github user ssimeonov commented on a diff in the pull request:
https://github.com/apache/spark/pull/21730#discussion_r200852355
--- Diff: python/pyspark/sql/conf.py ---
@@ -63,6 +63,12 @@ def _checkType(self, obj, identifier):
raise TypeError("expected %s '%s' to be a string (was '%s')" %
(identifier, obj, type(obj).__name__))
+ @ignore_unicode_prefix
+ @since(2.4)
+ def isModifiable(self, key):
+ """Is the configuration property modifiable or not."""
--- End diff --
Minor language tweak suggestion:
```
Indicates whether the configuration property with the given key is modifiable in the current session.
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21730: [SPARK-24761][SQL] Adding of isModifiable() to RuntimeCo...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21730
**[Test build #92886 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/92886/testReport)** for PR 21730 at commit [`d0d4620`](https://github.com/apache/spark/commit/d0d4620aa56f4d97a7899102dfeea601c2667e17).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21730: [SPARK-24761][SQL] Adding of isModifiable() to Ru...
Posted by MaxGekk <gi...@git.apache.org>.
Github user MaxGekk commented on a diff in the pull request:
https://github.com/apache/spark/pull/21730#discussion_r201817788
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/RuntimeConfig.scala ---
@@ -132,6 +132,14 @@ class RuntimeConfig private[sql](sqlConf: SQLConf = new SQLConf) {
sqlConf.unsetConf(key)
}
+ /**
+ * Indicates whether the configuration property with the given key
+ * is modifiable in the current session.
--- End diff --
I added `@return` tag with this info.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21730: [SPARK-24761][SQL] Adding of isModifiable() to RuntimeCo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21730
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21730: [SPARK-24761][SQL] Adding of isModifiable() to RuntimeCo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21730
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21730: [SPARK-24761][SQL] Adding of isModifiable() to RuntimeCo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21730
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21730: [SPARK-24761][SQL] Adding of isModifiable() to RuntimeCo...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21730
**[Test build #92722 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/92722/testReport)** for PR 21730 at commit [`465a5dd`](https://github.com/apache/spark/commit/465a5dd85a643e1cbc2b87f9d896e6053a51073d).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21730: [SPARK-24761][SQL] Adding of isModifiable() to RuntimeCo...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21730
**[Test build #92886 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/92886/testReport)** for PR 21730 at commit [`d0d4620`](https://github.com/apache/spark/commit/d0d4620aa56f4d97a7899102dfeea601c2667e17).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21730: [SPARK-24761][SQL] Adding of isModifiable() to RuntimeCo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21730
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/92886/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21730: [SPARK-24761][SQL] Adding of isModifiable() to Ru...
Posted by MaxGekk <gi...@git.apache.org>.
Github user MaxGekk commented on a diff in the pull request:
https://github.com/apache/spark/pull/21730#discussion_r201817629
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/RuntimeConfigSuite.scala ---
@@ -54,4 +54,15 @@ class RuntimeConfigSuite extends SparkFunSuite {
conf.get("k1")
}
}
+
+ test("SPARK-24761: is a config parameter modifiable") {
+ val conf = newConf()
+
+ // SQL configs
+ assert(!conf.isModifiable("spark.sql.sources.schemaStringLengthThreshold"))
--- End diff --
Added a couple tests
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21730: [SPARK-24761][SQL] Adding of isModifiable() to Ru...
Posted by gengliangwang <gi...@git.apache.org>.
Github user gengliangwang commented on a diff in the pull request:
https://github.com/apache/spark/pull/21730#discussion_r201677755
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/RuntimeConfig.scala ---
@@ -132,6 +132,14 @@ class RuntimeConfig private[sql](sqlConf: SQLConf = new SQLConf) {
sqlConf.unsetConf(key)
}
+ /**
+ * Indicates whether the configuration property with the given key
+ * is modifiable in the current session.
--- End diff --
nit: If a key is not a valid conf, this function will still return false. Should we specify this in the comments?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21730: [SPARK-24761][SQL] Adding of isModifiable() to Ru...
Posted by MaxGekk <gi...@git.apache.org>.
Github user MaxGekk commented on a diff in the pull request:
https://github.com/apache/spark/pull/21730#discussion_r200851372
--- Diff: python/pyspark/sql/conf.py ---
@@ -63,6 +63,12 @@ def _checkType(self, obj, identifier):
raise TypeError("expected %s '%s' to be a string (was '%s')" %
(identifier, obj, type(obj).__name__))
+ @ignore_unicode_prefix
+ @since(2.4)
+ def isModifiable(self, key):
+ """Is the configuration property modifiable or not."""
--- End diff --
From which language would you prefer the comment? or maybe the comment should contain more information? Please, let me know if something is not clear, and we should explain that in the comment.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21730: [SPARK-24761][SQL] Adding of isModifiable() to RuntimeCo...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21730
**[Test build #92722 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/92722/testReport)** for PR 21730 at commit [`465a5dd`](https://github.com/apache/spark/commit/465a5dd85a643e1cbc2b87f9d896e6053a51073d).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21730: [SPARK-24761][SQL] Adding of isModifiable() to RuntimeCo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21730
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/92722/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org