[GitHub] spark pull request #23136: [SPARK-25515][K8s] Adds a config option to keep e...

2018-11-29 Thread onursatici
Github user onursatici commented on a diff in the pull request: https://github.com/apache/spark/pull/23136#discussion_r237534100 --- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsAllocator.scala --- @@ -86,11 +88,14

[GitHub] spark pull request #23136: [SPARK-25515][K8s] Adds a config option to keep e...

2018-11-29 Thread onursatici
Github user onursatici commented on a diff in the pull request: https://github.com/apache/spark/pull/23136#discussion_r237534125 --- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsLifecycleManager.scala --- @@ -30,7 +30,7

[GitHub] spark issue #22146: [SPARK-24434][K8S] pod template files

2018-10-17 Thread onursatici
Github user onursatici commented on the issue: https://github.com/apache/spark/pull/22146 @ifilonenko I would like to get this merged as well, I think the only blocker for that is for us to decide how to specify the spark container in the pod template. @erikerlandson @mccheah

[GitHub] spark issue #22146: [SPARK-24434][K8S] pod template files

2018-09-10 Thread onursatici
Github user onursatici commented on the issue: https://github.com/apache/spark/pull/22146 @mccheah @erikerlandson @skonto should we keep the container selection logic here, or should we revert back to having a configuration value for pinning spark container names, either by name

[GitHub] spark pull request #22146: [SPARK-24434][K8S] pod template files

2018-08-31 Thread onursatici
Github user onursatici commented on a diff in the pull request: https://github.com/apache/spark/pull/22146#discussion_r214339066 --- Diff: docs/running-on-kubernetes.md --- @@ -185,6 +185,21 @@ To use a secret through an environment variable use the following options

[GitHub] spark pull request #22256: [SPARK-25262][K8S] Better support configurability...

2018-08-30 Thread onursatici
Github user onursatici commented on a diff in the pull request: https://github.com/apache/spark/pull/22256#discussion_r214198366 --- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/LocalDirsFeatureStep.scala --- @@ -37,41 +40,99

[GitHub] spark issue #22146: [SPARK-24434][K8S] pod template files

2018-08-30 Thread onursatici
Github user onursatici commented on the issue: https://github.com/apache/spark/pull/22146 @tnachen I have added a test for additional volumes here: https://github.com/apache/spark/pull/22146/commits/8b8aa48927aa35ba3683ea7eaed093a570721143 and this seems to pass locally

[GitHub] spark issue #22146: [SPARK-24434][K8S] pod template files

2018-08-29 Thread onursatici
Github user onursatici commented on the issue: https://github.com/apache/spark/pull/22146 @mccheah @erikerlandson @liyinan926 @ifilonenko @skonto this pr is ready for a final review. I am only planning to add one integration test for malformed templates, if I can find a non intrusive

[GitHub] spark pull request #22146: [SPARK-24434][K8S] pod template files

2018-08-29 Thread onursatici
Github user onursatici commented on a diff in the pull request: https://github.com/apache/spark/pull/22146#discussion_r213846434 --- Diff: resource-managers/kubernetes/core/src/test/scala/org/apache/spark/deploy/k8s/features/PodTemplateConfigMapStepSuite.scala --- @@ -0,0 +1,96

[GitHub] spark pull request #22146: [SPARK-24434][K8S] pod template files

2018-08-29 Thread onursatici
Github user onursatici commented on a diff in the pull request: https://github.com/apache/spark/pull/22146#discussion_r213817812 --- Diff: docs/running-on-kubernetes.md --- @@ -185,6 +185,21 @@ To use a secret through an environment variable use the following options

[GitHub] spark pull request #22146: [WIP][SPARK-24434][K8S] pod template files

2018-08-19 Thread onursatici
GitHub user onursatici opened a pull request: https://github.com/apache/spark/pull/22146 [WIP][SPARK-24434][K8S] pod template files ## What changes were proposed in this pull request? New feature to pass podspec files for driver and executor pods. ## How

[GitHub] spark pull request #21872: [WIP] merge upstream

2018-07-25 Thread onursatici
Github user onursatici closed the pull request at: https://github.com/apache/spark/pull/21872 --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h

[GitHub] spark pull request #21872: [WIP] merge upstream

2018-07-25 Thread onursatici
GitHub user onursatici opened a pull request: https://github.com/apache/spark/pull/21872 [WIP] merge upstream ## What changes were proposed in this pull request? (Please fill in changes proposed in this fix) ## How was this patch tested? (Please explain

[GitHub] spark issue #21805: [SPARK-24850][SQL] fix str representation of CachedRDDBu...

2018-07-23 Thread onursatici
Github user onursatici commented on the issue: https://github.com/apache/spark/pull/21805 @gatorsmile @maropu I have removed `batchSize` and `useCompression`, other than that the string representation is now the same as what we had on 2.3

[GitHub] spark pull request #21805: [SPARK-24850][SQL] fix str representation of Cach...

2018-07-20 Thread onursatici
Github user onursatici commented on a diff in the pull request: https://github.com/apache/spark/pull/21805#discussion_r204001527 --- Diff: sql/core/src/test/scala/org/apache/spark/sql/DatasetCacheSuite.scala --- @@ -206,4 +206,19 @@ class DatasetCacheSuite extends QueryTest

[GitHub] spark pull request #21805: [SPARK-24850][SQL] fix str representation of Cach...

2018-07-20 Thread onursatici
Github user onursatici commented on a diff in the pull request: https://github.com/apache/spark/pull/21805#discussion_r204001546 --- Diff: sql/core/src/test/scala/org/apache/spark/sql/DatasetCacheSuite.scala --- @@ -206,4 +206,19 @@ class DatasetCacheSuite extends QueryTest

[GitHub] spark pull request #21805: [SPARK-24850][SQL] fix str representation of Cach...

2018-07-18 Thread onursatici
GitHub user onursatici opened a pull request: https://github.com/apache/spark/pull/21805 [SPARK-24850][SQL] fix str representation of CachedRDDBuilder ## What changes were proposed in this pull request? As of https://github.com/apache/spark/pull/21018, InMemoryRelation includes

[GitHub] spark issue #21158: [SPARK-23850][sql] Add separate config for SQL options r...

2018-05-16 Thread onursatici
Github user onursatici commented on the issue: https://github.com/apache/spark/pull/21158 I don't have any strong opinions about usernames, so merging this as it stays is fine by me --- - To unsubscribe, e-mail

[GitHub] spark issue #21158: [SPARK-23850][sql] Add separate config for SQL options r...

2018-05-04 Thread onursatici
Github user onursatici commented on the issue: https://github.com/apache/spark/pull/21158 I think this PR is fine given it preserves the redaction of URL's. That was indeed the main reason why url's were in the default redaction pattern in the previous PR. To keep all

[GitHub] spark pull request #19761: [SPARK-22479][SQL][BRANCH-2.2] Exclude credential...

2017-11-20 Thread onursatici
Github user onursatici closed the pull request at: https://github.com/apache/spark/pull/19761 --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h

[GitHub] spark issue #19761: [SPARK-22479][SQL][BRANCH-2.2] Exclude credentials from ...

2017-11-15 Thread onursatici
Github user onursatici commented on the issue: https://github.com/apache/spark/pull/19761 follow-up of https://github.com/apache/spark/pull/19708 --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org

[GitHub] spark pull request #19761: [SPARK-22479][SQL][BRANCH-2.2] Exclude credential...

2017-11-15 Thread onursatici
GitHub user onursatici opened a pull request: https://github.com/apache/spark/pull/19761 [SPARK-22479][SQL][BRANCH-2.2] Exclude credentials from SaveintoDataSourceCommand.simpleString ## What changes were proposed in this pull request? Do not include jdbc properties which

[GitHub] spark pull request #19708: [SPARK-22479][SQL] Exclude credentials from Savei...

2017-11-15 Thread onursatici
Github user onursatici commented on a diff in the pull request: https://github.com/apache/spark/pull/19708#discussion_r151131021 --- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/SaveIntoDataSourceCommandSuite.scala --- @@ -0,0 +1,48

[GitHub] spark issue #19708: [WIP][SPARK-22479][SQL] Exclude credentials from Saveint...

2017-11-14 Thread onursatici
Github user onursatici commented on the issue: https://github.com/apache/spark/pull/19708 Yeah PR description reflects the latest changes --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org

[GitHub] spark pull request #19708: [SPARK-22479][SQL] Exclude credentials from Savei...

2017-11-10 Thread onursatici
Github user onursatici commented on a diff in the pull request: https://github.com/apache/spark/pull/19708#discussion_r150287784 --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/SaveIntoDataSourceCommand.scala --- @@ -46,4 +46,6 @@ case class

[GitHub] spark pull request #19708: [SPARK-22479][SQL] Exclude credentials from Savei...

2017-11-09 Thread onursatici
GitHub user onursatici opened a pull request: https://github.com/apache/spark/pull/19708 [SPARK-22479][SQL] Exclude credentials from SaveintoDataSourceCommand.simpleString ## What changes were proposed in this pull request? Do not include jdbc properties which may contain