GitHub PullRequestReviewCommentEvent at 2014-09-24 20:43:25
Repository: spark
Actor: vanzin
Payload: {"action":"created","comment":{"url":"https://api.github.com/repos/apache/spark/pulls/comments/17998933","id":17998933,"diff_hunk":"@@ -0,0 +1,38 @@\n+/*\n+ * Licensed to the Apache Software Foundation (ASF) under one or more\n+ * contributor license agreements.  See the NOTICE file distributed with\n+ * this work for additional information regarding copyright ownership.\n+ * The ASF licenses this file to You under the Apache License, Version 2.0\n+ * (the \"License\"); you may not use this file except in compliance with\n+ * the License.  You may obtain a copy of the License at\n+ *\n+ *    http://www.apache.org/licenses/LICENSE-2.0\n+ *\n+ * Unless required by applicable law or agreed to in writing, software\n+ * distributed under the License is distributed on an \"AS IS\" BASIS,\n+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n+ * See the License for the specific language governing permissions and\n+ * limitations under the License.\n+ */\n+package org.apache.spark.deploy\n+\n+import scala.collection._\n+\n+\n+object MergedPropertyMap {","path":"core/src/main/scala/org/apache/spark/deploy/MergedPropertyMap.scala","position":22,"original_position":22,"commit_id":"b69f58e65d919a689942866f59b11a7dcf2fbf91","original_commit_id":"b69f58e65d919a689942866f59b11a7dcf2fbf91","user":{"login":"vanzin","id":1694083,"avatar_url":"https://avatars.githubusercontent.com/u/1694083?v=2","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},"body":"Why this object and not a new method in Utils.scala?\r\n\r\nAt the very, very least this should be `private[spark]`.","created_at":"2014-09-24T20:43:25Z","updated_at":"2014-09-24T20:43:25Z","html_url":"https://github.com/apache/spark/pull/2516#discussion_r17998933","pull_request_url":"https://api.github.com/repos/apache/spark/pulls/2516","_links":{"self":{"href":"https://api.github.com/repos/apache/spark/pulls/comments/17998933"},"html":{"href":"https://github.com/apache/spark/pull/2516#discussion_r17998933"},"pull_request":{"href":"https://api.github.com/repos/apache/spark/pulls/2516"}}},"pull_request":{"url":"https://api.github.com/repos/apache/spark/pulls/2516","id":21696362,"html_url":"https://github.com/apache/spark/pull/2516","diff_url":"https://github.com/apache/spark/pull/2516.diff","patch_url":"https://github.com/apache/spark/pull/2516.patch","issue_url":"https://api.github.com/repos/apache/spark/issues/2516","number":2516,"state":"open","locked":false,"title":"Spark Core - [SPARK-3620] - Refactor of SparkSubmit Argument parsing code","user":{"login":"tigerquoll","id":8340922,"avatar_url":"https://avatars.githubusercontent.com/u/8340922?v=2","gravatar_id":"","url":"https://api.github.com/users/tigerquoll","html_url":"https://github.com/tigerquoll","followers_url":"https://api.github.com/users/tigerquoll/followers","following_url":"https://api.github.com/users/tigerquoll/following{/other_user}","gists_url":"https://api.github.com/users/tigerquoll/gists{/gist_id}","starred_url":"https://api.github.com/users/tigerquoll/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tigerquoll/subscriptions","organizations_url":"https://api.github.com/users/tigerquoll/orgs","repos_url":"https://api.github.com/users/tigerquoll/repos","events_url":"https://api.github.com/users/tigerquoll/events{/privacy}","received_events_url":"https://api.github.com/users/tigerquoll/received_events","type":"User","site_admin":false},"body":"Argument processing seems to have gotten a lot of attention lately, so I thought I might throw my contribution into the ring.  Attached for consideration and to prompt discussion is a revamp of argument handling in SparkSubmit aimed at making things a lot more consistent. The only things that have been modified are the way that configuration properties are read/ processed and prioritised \r\n\r\nThings to note include:\r\n* All configuration parameters can now be consistently set via config file\r\n\r\n* Configuration parameters defaults have been removed from the code, and placed into a property file which is read from the class path on startup.  There should be no need to trace through 5 files to see what a config parameter defaults to if it is not specified, or have different default values applied in multiple places throughout the code.\r\n\r\n* Configuration parameter validation is now done once all configuration parameters have been read in and resolved from various locations, not just when reading the command line.\r\n\r\n* All property files (including spark_default_conf) are parsed by Java property handling code. All custom parsing code has been removed. Escaping of characters should now be consistent everywhere.\r\n\r\n* All configuration parameters are overridden in the same consistent way - configuration parameters for sparkSubmit are pulled form the following sources in order of priority\r\n 1. Entries specified on the command line (except from --conf entries)\r\n 2. Entries specified on the command line with --conf\r\n 3. Environment variables (including legacy variable mappings)\r\n 4. System config variables (eg by using -Dspark.var.name)\r\n 5. $(SPARK_DEFAULT_CONF)/spark-defaults.conf or $(SPARK_HOME)/conf/spark-defaults.conf if either exist\r\n 6. Hard coded defaults in class path at spark-submit-defaults.prop\r\n\r\n* A property file specified by one of the sources listed above gets read in and the properties are considered to be at the priority of the configuration source that specified the file. A property specified in a property file will not override an existing config value already specifiedby that configuration source\r\n\r\nThe existing argument handling is pretty finicky - chances are high that I’ve missed some behaviour - if this PR is going to be accepted/approved let me know any bugs and I’ll fix them up and document the behaviour for future reference","created_at":"2014-09-24T07:28:22Z","updated_at":"2014-09-24T20:43:25Z","closed_at":null,"merged_at":null,"merge_commit_sha":"36420ab21f232ca7a7f9146d45590c3e18642f95","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/apache/spark/pulls/2516/commits","review_comments_url":"https://api.github.com/repos/apache/spark/pulls/2516/comments","review_comment_url":"https://api.github.com/repos/apache/spark/pulls/comments/{number}","comments_url":"https://api.github.com/repos/apache/spark/issues/2516/comments","statuses_url":"https://api.github.com/repos/apache/spark/statuses/b69f58e65d919a689942866f59b11a7dcf2fbf91","head":{"label":"tigerquoll:master","ref":"master","sha":"b69f58e65d919a689942866f59b11a7dcf2fbf91","user":{"login":"tigerquoll","id":8340922,"avatar_url":"https://avatars.githubusercontent.com/u/8340922?v=2","gravatar_id":"","url":"https://api.github.com/users/tigerquoll","html_url":"https://github.com/tigerquoll","followers_url":"https://api.github.com/users/tigerquoll/followers","following_url":"https://api.github.com/users/tigerquoll/following{/other_user}","gists_url":"https://api.github.com/users/tigerquoll/gists{/gist_id}","starred_url":"https://api.github.com/users/tigerquoll/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tigerquoll/subscriptions","organizations_url":"https://api.github.com/users/tigerquoll/orgs","repos_url":"https://api.github.com/users/tigerquoll/repos","events_url":"https://api.github.com/users/tigerquoll/events{/privacy}","received_events_url":"https://api.github.com/users/tigerquoll/received_events","type":"User","site_admin":false},"repo":{"id":24401129,"name":"spark-3620","full_name":"tigerquoll/spark-3620","owner":{"login":"tigerquoll","id":8340922,"avatar_url":"https://avatars.githubusercontent.com/u/8340922?v=2","gravatar_id":"","url":"https://api.github.com/users/tigerquoll","html_url":"https://github.com/tigerquoll","followers_url":"https://api.github.com/users/tigerquoll/followers","following_url":"https://api.github.com/users/tigerquoll/following{/other_user}","gists_url":"https://api.github.com/users/tigerquoll/gists{/gist_id}","starred_url":"https://api.github.com/users/tigerquoll/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tigerquoll/subscriptions","organizations_url":"https://api.github.com/users/tigerquoll/orgs","repos_url":"https://api.github.com/users/tigerquoll/repos","events_url":"https://api.github.com/users/tigerquoll/events{/privacy}","received_events_url":"https://api.github.com/users/tigerquoll/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/tigerquoll/spark-3620","description":"Mirror of Apache Spark","fork":true,"url":"https://api.github.com/repos/tigerquoll/spark-3620","forks_url":"https://api.github.com/repos/tigerquoll/spark-3620/forks","keys_url":"https://api.github.com/repos/tigerquoll/spark-3620/keys{/key_id}","collaborators_url":"https://api.github.com/repos/tigerquoll/spark-3620/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/tigerquoll/spark-3620/teams","hooks_url":"https://api.github.com/repos/tigerquoll/spark-3620/hooks","issue_events_url":"https://api.github.com/repos/tigerquoll/spark-3620/issues/events{/number}","events_url":"https://api.github.com/repos/tigerquoll/spark-3620/events","assignees_url":"https://api.github.com/repos/tigerquoll/spark-3620/assignees{/user}","branches_url":"https://api.github.com/repos/tigerquoll/spark-3620/branches{/branch}","tags_url":"https://api.github.com/repos/tigerquoll/spark-3620/tags","blobs_url":"https://api.github.com/repos/tigerquoll/spark-3620/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/tigerquoll/spark-3620/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/tigerquoll/spark-3620/git/refs{/sha}","trees_url":"https://api.github.com/repos/tigerquoll/spark-3620/git/trees{/sha}","statuses_url":"https://api.github.com/repos/tigerquoll/spark-3620/statuses/{sha}","languages_url":"https://api.github.com/repos/tigerquoll/spark-3620/languages","stargazers_url":"https://api.github.com/repos/tigerquoll/spark-3620/stargazers","contributors_url":"https://api.github.com/repos/tigerquoll/spark-3620/contributors","subscribers_url":"https://api.github.com/repos/tigerquoll/spark-3620/subscribers","subscription_url":"https://api.github.com/repos/tigerquoll/spark-3620/subscription","commits_url":"https://api.github.com/repos/tigerquoll/spark-3620/commits{/sha}","git_commits_url":"https://api.github.com/repos/tigerquoll/spark-3620/git/commits{/sha}","comments_url":"https://api.github.com/repos/tigerquoll/spark-3620/comments{/number}","issue_comment_url":"https://api.github.com/repos/tigerquoll/spark-3620/issues/comments/{number}","contents_url":"https://api.github.com/repos/tigerquoll/spark-3620/contents/{+path}","compare_url":"https://api.github.com/repos/tigerquoll/spark-3620/compare/{base}...{head}","merges_url":"https://api.github.com/repos/tigerquoll/spark-3620/merges","archive_url":"https://api.github.com/repos/tigerquoll/spark-3620/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/tigerquoll/spark-3620/downloads","issues_url":"https://api.github.com/repos/tigerquoll/spark-3620/issues{/number}","pulls_url":"https://api.github.com/repos/tigerquoll/spark-3620/pulls{/number}","milestones_url":"https://api.github.com/repos/tigerquoll/spark-3620/milestones{/number}","notifications_url":"https://api.github.com/repos/tigerquoll/spark-3620/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/tigerquoll/spark-3620/labels{/name}","releases_url":"https://api.github.com/repos/tigerquoll/spark-3620/releases{/id}","created_at":"2014-09-24T04:48:11Z","updated_at":"2014-09-24T06:05:09Z","pushed_at":"2014-09-24T07:08:33Z","git_url":"git://github.com/tigerquoll/spark-3620.git","ssh_url":"git@github.com:tigerquoll/spark-3620.git","clone_url":"https://github.com/tigerquoll/spark-3620.git","svn_url":"https://github.com/tigerquoll/spark-3620","homepage":null,"size":77769,"stargazers_count":0,"watchers_count":0,"language":"Scala","has_issues":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"apache:master","ref":"master","sha":"729952a5efce755387c76cdf29280ee6f49fdb72","user":{"login":"apache","id":47359,"avatar_url":"https://avatars.githubusercontent.com/u/47359?v=2","gravatar_id":"","url":"https://api.github.com/users/apache","html_url":"https://github.com/apache","followers_url":"https://api.github.com/users/apache/followers","following_url":"https://api.github.com/users/apache/following{/other_user}","gists_url":"https://api.github.com/users/apache/gists{/gist_id}","starred_url":"https://api.github.com/users/apache/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/apache/subscriptions","organizations_url":"https://api.github.com/users/apache/orgs","repos_url":"https://api.github.com/users/apache/repos","events_url":"https://api.github.com/users/apache/events{/privacy}","received_events_url":"https://api.github.com/users/apache/received_events","type":"Organization","site_admin":false},"repo":{"id":17165658,"name":"spark","full_name":"apache/spark","owner":{"login":"apache","id":47359,"avatar_url":"https://avatars.githubusercontent.com/u/47359?v=2","gravatar_id":"","url":"https://api.github.com/users/apache","html_url":"https://github.com/apache","followers_url":"https://api.github.com/users/apache/followers","following_url":"https://api.github.com/users/apache/following{/other_user}","gists_url":"https://api.github.com/users/apache/gists{/gist_id}","starred_url":"https://api.github.com/users/apache/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/apache/subscriptions","organizations_url":"https://api.github.com/users/apache/orgs","repos_url":"https://api.github.com/users/apache/repos","events_url":"https://api.github.com/users/apache/events{/privacy}","received_events_url":"https://api.github.com/users/apache/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/apache/spark","description":"Mirror of Apache Spark","fork":false,"url":"https://api.github.com/repos/apache/spark","forks_url":"https://api.github.com/repos/apache/spark/forks","keys_url":"https://api.github.com/repos/apache/spark/keys{/key_id}","collaborators_url":"https://api.github.com/repos/apache/spark/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/apache/spark/teams","hooks_url":"https://api.github.com/repos/apache/spark/hooks","issue_events_url":"https://api.github.com/repos/apache/spark/issues/events{/number}","events_url":"https://api.github.com/repos/apache/spark/events","assignees_url":"https://api.github.com/repos/apache/spark/assignees{/user}","branches_url":"https://api.github.com/repos/apache/spark/branches{/branch}","tags_url":"https://api.github.com/repos/apache/spark/tags","blobs_url":"https://api.github.com/repos/apache/spark/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/apache/spark/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/apache/spark/git/refs{/sha}","trees_url":"https://api.github.com/repos/apache/spark/git/trees{/sha}","statuses_url":"https://api.github.com/repos/apache/spark/statuses/{sha}","languages_url":"https://api.github.com/repos/apache/spark/languages","stargazers_url":"https://api.github.com/repos/apache/spark/stargazers","contributors_url":"https://api.github.com/repos/apache/spark/contributors","subscribers_url":"https://api.github.com/repos/apache/spark/subscribers","subscription_url":"https://api.github.com/repos/apache/spark/subscription","commits_url":"https://api.github.com/repos/apache/spark/commits{/sha}","git_commits_url":"https://api.github.com/repos/apache/spark/git/commits{/sha}","comments_url":"https://api.github.com/repos/apache/spark/comments{/number}","issue_comment_url":"https://api.github.com/repos/apache/spark/issues/comments/{number}","contents_url":"https://api.github.com/repos/apache/spark/contents/{+path}","compare_url":"https://api.github.com/repos/apache/spark/compare/{base}...{head}","merges_url":"https://api.github.com/repos/apache/spark/merges","archive_url":"https://api.github.com/repos/apache/spark/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/apache/spark/downloads","issues_url":"https://api.github.com/repos/apache/spark/issues{/number}","pulls_url":"https://api.github.com/repos/apache/spark/pulls{/number}","milestones_url":"https://api.github.com/repos/apache/spark/milestones{/number}","notifications_url":"https://api.github.com/repos/apache/spark/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/apache/spark/labels{/name}","releases_url":"https://api.github.com/repos/apache/spark/releases{/id}","created_at":"2014-02-25T08:00:08Z","updated_at":"2014-09-24T15:25:16Z","pushed_at":"2014-09-24T20:11:14Z","git_url":"git://github.com/apache/spark.git","ssh_url":"git@github.com:apache/spark.git","clone_url":"https://github.com/apache/spark.git","svn_url":"https://github.com/apache/spark","homepage":null,"size":3263282,"stargazers_count":1551,"watchers_count":1551,"language":"Scala","has_issues":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":1378,"mirror_url":"git://git.apache.org/spark.git","open_issues_count":251,"forks":1378,"open_issues":251,"watchers":1551,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/apache/spark/pulls/2516"},"html":{"href":"https://github.com/apache/spark/pull/2516"},"issue":{"href":"https://api.github.com/repos/apache/spark/issues/2516"},"comments":{"href":"https://api.github.com/repos/apache/spark/issues/2516/comments"},"review_comments":{"href":"https://api.github.com/repos/apache/spark/pulls/2516/comments"},"review_comment":{"href":"https://api.github.com/repos/apache/spark/pulls/comments/{number}"},"commits":{"href":"https://api.github.com/repos/apache/spark/pulls/2516/commits"},"statuses":{"href":"https://api.github.com/repos/apache/spark/statuses/b69f58e65d919a689942866f59b11a7dcf2fbf91"}}}}