{"action":"created","issue":{"url":"https://api.github.com/repos/apache/spark/issues/22897","repository_url":"https://api.github.com/repos/apache/spark","labels_url":"https://api.github.com/repos/apache/spark/issues/22897/labels{/name}","comments_url":"https://api.github.com/repos/apache/spark/issues/22897/comments","events_url":"https://api.github.com/repos/apache/spark/issues/22897/events","html_url":"https://github.com/apache/spark/pull/22897","id":375639793,"node_id":"MDExOlB1bGxSZXF1ZXN0MjI3MDM5NDc4","number":22897,"title":"[SPARK-25875][k8s] Merge code to set up driver command into a single step.","user":{"login":"vanzin","id":1694083,"node_id":"MDQ6VXNlcjE2OTQwODM=","avatar_url":"https://avatars0.githubusercontent.com/u/1694083?v=4","gravatar_id":"","url":"https://api.github.com/users/vanzin","html_url":"https://github.com/vanzin","followers_url":"https://api.github.com/users/vanzin/followers","following_url":"https://api.github.com/users/vanzin/following{/other_user}","gists_url":"https://api.github.com/users/vanzin/gists{/gist_id}","starred_url":"https://api.github.com/users/vanzin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/vanzin/subscriptions","organizations_url":"https://api.github.com/users/vanzin/orgs","repos_url":"https://api.github.com/users/vanzin/repos","events_url":"https://api.github.com/users/vanzin/events{/privacy}","received_events_url":"https://api.github.com/users/vanzin/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":26,"created_at":"2018-10-30T18:45:41Z","updated_at":"2018-11-02T18:09:05Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/apache/spark/pulls/22897","html_url":"https://github.com/apache/spark/pull/22897","diff_url":"https://github.com/apache/spark/pull/22897.diff","patch_url":"https://github.com/apache/spark/pull/22897.patch"},"body":"Right now there are 3 different classes dealing with building the driver\r\ncommand to run inside the pod, one for each \"binding\" supported by Spark.\r\nThis has two main shortcomings:\r\n\r\n- the code in the 3 classes is very similar; changing things in one place\r\n  would probably mean making a similar change in the others.\r\n\r\n- it gives the false impression that the step implementation is the only\r\n  place where binding-specific logic is needed. That is not true; there\r\n  was code in KubernetesConf that was binding-specific, and there's also\r\n  code in the executor-specific config step. So the 3 classes weren't really\r\n  working as a language-specific abstraction.\r\n\r\nOn top of that, the current code was propagating command line parameters in\r\na different way depending on the binding. That doesn't seem necessary, and\r\nin fact using environment variables for command line parameters is in general\r\na really bad idea, since you can't handle special characters (e.g. spaces)\r\nthat way.\r\n\r\nThis change merges the 3 different code paths for Java, Python and R into\r\na single step, and also merges the 3 code paths to start the Spark driver\r\nin the k8s entry point script. This increases the amount of shared code,\r\nand also moves more feature logic into the step itself, so it doesn't live\r\nin KubernetesConf.\r\n\r\nNote that not all logic related to setting up the driver lives in that\r\nstep. For example, the memory overhead calculation still lives separately,\r\nexcept it now happens in the driver config step instead of outside the\r\nstep hierarchy altogether.\r\n\r\nSome of the noise in the diff is because of changes to KubernetesConf, which\r\nwill be addressed in a separate change.\r\n\r\nTested with new and updated unit tests + integration tests.\r\n"},"comment":{"url":"https://api.github.com/repos/apache/spark/issues/comments/435463578","html_url":"https://github.com/apache/spark/pull/22897#issuecomment-435463578","issue_url":"https://api.github.com/repos/apache/spark/issues/22897","id":435463578,"node_id":"MDEyOklzc3VlQ29tbWVudDQzNTQ2MzU3OA==","user":{"login":"SparkQA","id":8108735,"node_id":"MDQ6VXNlcjgxMDg3MzU=","avatar_url":"https://avatars1.githubusercontent.com/u/8108735?v=4","gravatar_id":"","url":"https://api.github.com/users/SparkQA","html_url":"https://github.com/SparkQA","followers_url":"https://api.github.com/users/SparkQA/followers","following_url":"https://api.github.com/users/SparkQA/following{/other_user}","gists_url":"https://api.github.com/users/SparkQA/gists{/gist_id}","starred_url":"https://api.github.com/users/SparkQA/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/SparkQA/subscriptions","organizations_url":"https://api.github.com/users/SparkQA/orgs","repos_url":"https://api.github.com/users/SparkQA/repos","events_url":"https://api.github.com/users/SparkQA/events{/privacy}","received_events_url":"https://api.github.com/users/SparkQA/received_events","type":"User","site_admin":false},"created_at":"2018-11-02T18:09:04Z","updated_at":"2018-11-02T18:09:04Z","author_association":"NONE","body":"**[Test build #98403 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98403/testReport)** for PR 22897 at commit [`0d755d6`](https://github.com/apache/spark/commit/0d755d6549c46332ee7fa54c335d5c6c19727671).\n * This patch passes all tests.\n * This patch merges cleanly.\n * This patch adds no public classes."}}
{"actor":{"display_login":"SparkQA"}}