GitHub PullRequestReviewCommentEvent at 2014-10-08 22:32:02
Repository: cdap
Actor: chtyim
Payload: {"action":"created","comment":{"url":"https://api.github.com/repos/caskdata/cdap/pulls/comments/18617153","id":18617153,"diff_hunk":"@@ -81,81 +85,82 @@ public ProgramController run(Program program, ProgramOptions options) {\n \n     //RunId for the service\n     RunId runId = RunIds.generate();\n-    programOptions.put(runId, options);\n-    final Table<String, Integer, ProgramController> serviceRunnables = createRunnables(program, runId, serviceSpec);\n-    return new ServiceProgramController(serviceRunnables, runId, program, serviceSpec);\n+    Table<String, Integer, ProgramController> components = startAllComponents(program, runId,\n+                                                                              options.getUserArguments(), serviceSpec);\n+    return new ServiceProgramController(components, runId, program, serviceSpec, options.getUserArguments());\n   }\n \n-  private Table<String, Integer, ProgramController> createRunnables(Program program, RunId runId,\n-                                                                    ServiceSpecification serviceSpec) {\n-    Table<String, Integer, ProgramController> runnables = HashBasedTable.create();\n-\n-    try {\n-      for (Map.Entry<String, RuntimeSpecification> entry : serviceSpec.getRunnables().entrySet()) {\n-        int instanceCount = entry.getValue().getResourceSpecification().getInstances();\n-        for (int instanceId = 0; instanceId < instanceCount; instanceId++) {\n-          runnables.put(entry.getKey(), instanceId, startRunnable\n-            (program, createRunnableOptions(entry.getKey(), instanceId, instanceCount, runId)));\n-        }\n-      }\n-    } catch (Throwable t) {\n-      // Need to stop all started runnable here.\n-      try {\n-        Futures.successfulAsList\n-          (Iterables.transform(runnables.values(), new Function<ProgramController,\n-                                 ListenableFuture<ProgramController>>() {\n-                                 @Override\n-                                 public ListenableFuture<ProgramController> apply(ProgramController input) {\n-                                   return input.stop();\n-                                 }\n-                               }\n-           )\n-          ).get();\n-      } catch (Exception e) {\n-        LOG.error(\"Failed to stop all the runnables\");\n-      }\n-      throw Throwables.propagate(t);\n+  /**\n+   * Creates and starts all components of the given Service program.\n+   *\n+   * @return A {@link Table} with component name in the row, instance id in the column and the {@link ProgramController}\n+   *         of the component runner as the cell value.\n+   */\n+  private Table<String, Integer, ProgramController> startAllComponents(Program program, RunId runId,\n+                                                                       Arguments userArguments,\n+                                                                       ServiceSpecification spec) {\n+    Table<String, Integer, ProgramController> components = HashBasedTable.create();\n+\n+    // Starts the http service. The name is the same as the service name.\n+    startComponent(program, program.getName(), spec.getInstances(), runId, userArguments, components);\n+\n+    // Starts all Workers\n+    for (Map.Entry<String, ServiceWorkerSpecification> entry : spec.getWorkers().entrySet()) {\n+      ServiceWorkerSpecification workerSpec = entry.getValue();\n+      startComponent(program, workerSpec.getName(), workerSpec.getInstances(), runId, userArguments, components);\n     }\n-    return runnables;\n-  }\n \n-  private ProgramController startRunnable(Program program, ProgramOptions runnableOptions) {\n-    return programRunnerFactory.create(ProgramRunnerFactory.Type.RUNNABLE).run(program, runnableOptions);\n+    return components;\n   }\n \n-  private ProgramOptions createRunnableOptions(String name, int instanceId, int instances, RunId runId) {\n-\n-    // Get the right user arguments.\n-    Arguments userArguments = new BasicArguments();\n-    if (programOptions.containsKey(runId)) {\n-      userArguments = programOptions.get(runId).getUserArguments();\n+  /**\n+   * Starts all instances of a Service component.\n+   *\n+   * @param program The program to run\n+   * @param name Name of the service component\n+   * @param instances Number of instances to start\n+   * @param runId The runId\n+   * @param userArguments User runtime arguments\n+   * @param components A Table for storing the resulting ProgramControllers\n+   */\n+  private void startComponent(Program program, String name, int instances, RunId runId, Arguments userArguments,\n+                              Table<String, Integer, ProgramController> components) {\n+    for (int instanceId = 0; instanceId < instances; instanceId++) {\n+      ProgramOptions componentOptions = createComponentOptions(name, instanceId, instances, runId, userArguments);\n+      ProgramController controller = programRunnerFactory.create(ProgramRunnerFactory.Type.SERVICE_COMPONENT)\n+        .run(program, componentOptions);\n+      components.put(program.getName(), instanceId, controller);\n     }\n+  }\n \n-    return new SimpleProgramOptions(name, new BasicArguments\n-      (ImmutableMap.of(ProgramOptionConstants.INSTANCE_ID, Integer.toString(instanceId),\n-                       ProgramOptionConstants.INSTANCES, Integer.toString(instances),\n-                       ProgramOptionConstants.RUN_ID, runId.getId())), userArguments);\n+  private ProgramOptions createComponentOptions(String name, int instanceId,\n+                                                int instances, RunId runId, Arguments userArguments) {\n+    return new SimpleProgramOptions(name, new BasicArguments(\n+      ImmutableMap.of(ProgramOptionConstants.INSTANCE_ID, Integer.toString(instanceId),\n+                      ProgramOptionConstants.INSTANCES, Integer.toString(instances),\n+                      ProgramOptionConstants.RUN_ID, runId.getId(),\n+                      ProgramOptionConstants.HOST, host)\n+    ), userArguments);","path":"cdap-app-fabric/src/main/java/co/cask/cdap/internal/app/runtime/service/ServiceProgramRunner.java","position":null,"original_position":168,"commit_id":"3ee74b1b2b2dad174ebe4ff01ee5b3c690e48f7e","original_commit_id":"d70008aaa21e7d407f72eac84ef351cfa19bc55a","user":{"login":"chtyim","id":885032,"avatar_url":"https://avatars.githubusercontent.com/u/885032?v=2","gravatar_id":"","url":"https://api.github.com/users/chtyim","html_url":"https://github.com/chtyim","followers_url":"https://api.github.com/users/chtyim/followers","following_url":"https://api.github.com/users/chtyim/following{/other_user}","gists_url":"https://api.github.com/users/chtyim/gists{/gist_id}","starred_url":"https://api.github.com/users/chtyim/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/chtyim/subscriptions","organizations_url":"https://api.github.com/users/chtyim/orgs","repos_url":"https://api.github.com/users/chtyim/repos","events_url":"https://api.github.com/users/chtyim/events{/privacy}","received_events_url":"https://api.github.com/users/chtyim/received_events","type":"User","site_admin":false},"body":"Reformatted the code a bit.","created_at":"2014-10-08T22:32:02Z","updated_at":"2014-10-08T22:32:02Z","html_url":"https://github.com/caskdata/cdap/pull/510#discussion_r18617153","pull_request_url":"https://api.github.com/repos/caskdata/cdap/pulls/510","_links":{"self":{"href":"https://api.github.com/repos/caskdata/cdap/pulls/comments/18617153"},"html":{"href":"https://github.com/caskdata/cdap/pull/510#discussion_r18617153"},"pull_request":{"href":"https://api.github.com/repos/caskdata/cdap/pulls/510"}}},"pull_request":{"url":"https://api.github.com/repos/caskdata/cdap/pulls/510","id":22326097,"html_url":"https://github.com/caskdata/cdap/pull/510","diff_url":"https://github.com/caskdata/cdap/pull/510.diff","patch_url":"https://github.com/caskdata/cdap/pull/510.patch","issue_url":"https://api.github.com/repos/caskdata/cdap/issues/510","number":510,"state":"open","locked":false,"title":"(CDAP-378) Refactor Service implementation","user":{"login":"chtyim","id":885032,"avatar_url":"https://avatars.githubusercontent.com/u/885032?v=2","gravatar_id":"","url":"https://api.github.com/users/chtyim","html_url":"https://github.com/chtyim","followers_url":"https://api.github.com/users/chtyim/followers","following_url":"https://api.github.com/users/chtyim/following{/other_user}","gists_url":"https://api.github.com/users/chtyim/gists{/gist_id}","starred_url":"https://api.github.com/users/chtyim/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/chtyim/subscriptions","organizations_url":"https://api.github.com/users/chtyim/orgs","repos_url":"https://api.github.com/users/chtyim/repos","events_url":"https://api.github.com/users/chtyim/events{/privacy}","received_events_url":"https://api.github.com/users/chtyim/received_events","type":"User","site_admin":false},"body":"- Cleanup cluttered code about ProgramRunner implementation for Service\r\n  - Follow why other ProgramRunner model is\r\n- Refactor ServiceWorker API to use configurer.\r\n- Added support of @Property annotation","created_at":"2014-10-07T01:31:55Z","updated_at":"2014-10-08T22:32:02Z","closed_at":null,"merged_at":null,"merge_commit_sha":"46f7f870c1ea2261e9897eedc98cf87792e64103","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/caskdata/cdap/pulls/510/commits","review_comments_url":"https://api.github.com/repos/caskdata/cdap/pulls/510/comments","review_comment_url":"https://api.github.com/repos/caskdata/cdap/pulls/comments/{number}","comments_url":"https://api.github.com/repos/caskdata/cdap/issues/510/comments","statuses_url":"https://api.github.com/repos/caskdata/cdap/statuses/3ee74b1b2b2dad174ebe4ff01ee5b3c690e48f7e","head":{"label":"caskdata:feature/unify-api","ref":"feature/unify-api","sha":"3ee74b1b2b2dad174ebe4ff01ee5b3c690e48f7e","user":{"login":"caskdata","id":8123023,"avatar_url":"https://avatars.githubusercontent.com/u/8123023?v=2","gravatar_id":"","url":"https://api.github.com/users/caskdata","html_url":"https://github.com/caskdata","followers_url":"https://api.github.com/users/caskdata/followers","following_url":"https://api.github.com/users/caskdata/following{/other_user}","gists_url":"https://api.github.com/users/caskdata/gists{/gist_id}","starred_url":"https://api.github.com/users/caskdata/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/caskdata/subscriptions","organizations_url":"https://api.github.com/users/caskdata/orgs","repos_url":"https://api.github.com/users/caskdata/repos","events_url":"https://api.github.com/users/caskdata/events{/privacy}","received_events_url":"https://api.github.com/users/caskdata/received_events","type":"Organization","site_admin":false},"repo":{"id":22542759,"name":"cdap","full_name":"caskdata/cdap","owner":{"login":"caskdata","id":8123023,"avatar_url":"https://avatars.githubusercontent.com/u/8123023?v=2","gravatar_id":"","url":"https://api.github.com/users/caskdata","html_url":"https://github.com/caskdata","followers_url":"https://api.github.com/users/caskdata/followers","following_url":"https://api.github.com/users/caskdata/following{/other_user}","gists_url":"https://api.github.com/users/caskdata/gists{/gist_id}","starred_url":"https://api.github.com/users/caskdata/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/caskdata/subscriptions","organizations_url":"https://api.github.com/users/caskdata/orgs","repos_url":"https://api.github.com/users/caskdata/repos","events_url":"https://api.github.com/users/caskdata/events{/privacy}","received_events_url":"https://api.github.com/users/caskdata/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/caskdata/cdap","description":"Cask Data Application Platform (CDAP)","fork":false,"url":"https://api.github.com/repos/caskdata/cdap","forks_url":"https://api.github.com/repos/caskdata/cdap/forks","keys_url":"https://api.github.com/repos/caskdata/cdap/keys{/key_id}","collaborators_url":"https://api.github.com/repos/caskdata/cdap/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/caskdata/cdap/teams","hooks_url":"https://api.github.com/repos/caskdata/cdap/hooks","issue_events_url":"https://api.github.com/repos/caskdata/cdap/issues/events{/number}","events_url":"https://api.github.com/repos/caskdata/cdap/events","assignees_url":"https://api.github.com/repos/caskdata/cdap/assignees{/user}","branches_url":"https://api.github.com/repos/caskdata/cdap/branches{/branch}","tags_url":"https://api.github.com/repos/caskdata/cdap/tags","blobs_url":"https://api.github.com/repos/caskdata/cdap/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/caskdata/cdap/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/caskdata/cdap/git/refs{/sha}","trees_url":"https://api.github.com/repos/caskdata/cdap/git/trees{/sha}","statuses_url":"https://api.github.com/repos/caskdata/cdap/statuses/{sha}","languages_url":"https://api.github.com/repos/caskdata/cdap/languages","stargazers_url":"https://api.github.com/repos/caskdata/cdap/stargazers","contributors_url":"https://api.github.com/repos/caskdata/cdap/contributors","subscribers_url":"https://api.github.com/repos/caskdata/cdap/subscribers","subscription_url":"https://api.github.com/repos/caskdata/cdap/subscription","commits_url":"https://api.github.com/repos/caskdata/cdap/commits{/sha}","git_commits_url":"https://api.github.com/repos/caskdata/cdap/git/commits{/sha}","comments_url":"https://api.github.com/repos/caskdata/cdap/comments{/number}","issue_comment_url":"https://api.github.com/repos/caskdata/cdap/issues/comments/{number}","contents_url":"https://api.github.com/repos/caskdata/cdap/contents/{+path}","compare_url":"https://api.github.com/repos/caskdata/cdap/compare/{base}...{head}","merges_url":"https://api.github.com/repos/caskdata/cdap/merges","archive_url":"https://api.github.com/repos/caskdata/cdap/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/caskdata/cdap/downloads","issues_url":"https://api.github.com/repos/caskdata/cdap/issues{/number}","pulls_url":"https://api.github.com/repos/caskdata/cdap/pulls{/number}","milestones_url":"https://api.github.com/repos/caskdata/cdap/milestones{/number}","notifications_url":"https://api.github.com/repos/caskdata/cdap/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/caskdata/cdap/labels{/name}","releases_url":"https://api.github.com/repos/caskdata/cdap/releases{/id}","created_at":"2014-08-02T09:00:10Z","updated_at":"2014-10-07T17:15:07Z","pushed_at":"2014-10-08T22:31:52Z","git_url":"git://github.com/caskdata/cdap.git","ssh_url":"git@github.com:caskdata/cdap.git","clone_url":"https://github.com/caskdata/cdap.git","svn_url":"https://github.com/caskdata/cdap","homepage":"http://cdap.io","size":380549,"stargazers_count":10,"watchers_count":10,"language":"Java","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":true,"forks_count":3,"mirror_url":null,"open_issues_count":9,"forks":3,"open_issues":9,"watchers":10,"default_branch":"develop"}},"base":{"label":"caskdata:develop","ref":"develop","sha":"118c70d8b3b439571f4a46219cd58b6d1d8adbe9","user":{"login":"caskdata","id":8123023,"avatar_url":"https://avatars.githubusercontent.com/u/8123023?v=2","gravatar_id":"","url":"https://api.github.com/users/caskdata","html_url":"https://github.com/caskdata","followers_url":"https://api.github.com/users/caskdata/followers","following_url":"https://api.github.com/users/caskdata/following{/other_user}","gists_url":"https://api.github.com/users/caskdata/gists{/gist_id}","starred_url":"https://api.github.com/users/caskdata/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/caskdata/subscriptions","organizations_url":"https://api.github.com/users/caskdata/orgs","repos_url":"https://api.github.com/users/caskdata/repos","events_url":"https://api.github.com/users/caskdata/events{/privacy}","received_events_url":"https://api.github.com/users/caskdata/received_events","type":"Organization","site_admin":false},"repo":{"id":22542759,"name":"cdap","full_name":"caskdata/cdap","owner":{"login":"caskdata","id":8123023,"avatar_url":"https://avatars.githubusercontent.com/u/8123023?v=2","gravatar_id":"","url":"https://api.github.com/users/caskdata","html_url":"https://github.com/caskdata","followers_url":"https://api.github.com/users/caskdata/followers","following_url":"https://api.github.com/users/caskdata/following{/other_user}","gists_url":"https://api.github.com/users/caskdata/gists{/gist_id}","starred_url":"https://api.github.com/users/caskdata/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/caskdata/subscriptions","organizations_url":"https://api.github.com/users/caskdata/orgs","repos_url":"https://api.github.com/users/caskdata/repos","events_url":"https://api.github.com/users/caskdata/events{/privacy}","received_events_url":"https://api.github.com/users/caskdata/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/caskdata/cdap","description":"Cask Data Application Platform (CDAP)","fork":false,"url":"https://api.github.com/repos/caskdata/cdap","forks_url":"https://api.github.com/repos/caskdata/cdap/forks","keys_url":"https://api.github.com/repos/caskdata/cdap/keys{/key_id}","collaborators_url":"https://api.github.com/repos/caskdata/cdap/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/caskdata/cdap/teams","hooks_url":"https://api.github.com/repos/caskdata/cdap/hooks","issue_events_url":"https://api.github.com/repos/caskdata/cdap/issues/events{/number}","events_url":"https://api.github.com/repos/caskdata/cdap/events","assignees_url":"https://api.github.com/repos/caskdata/cdap/assignees{/user}","branches_url":"https://api.github.com/repos/caskdata/cdap/branches{/branch}","tags_url":"https://api.github.com/repos/caskdata/cdap/tags","blobs_url":"https://api.github.com/repos/caskdata/cdap/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/caskdata/cdap/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/caskdata/cdap/git/refs{/sha}","trees_url":"https://api.github.com/repos/caskdata/cdap/git/trees{/sha}","statuses_url":"https://api.github.com/repos/caskdata/cdap/statuses/{sha}","languages_url":"https://api.github.com/repos/caskdata/cdap/languages","stargazers_url":"https://api.github.com/repos/caskdata/cdap/stargazers","contributors_url":"https://api.github.com/repos/caskdata/cdap/contributors","subscribers_url":"https://api.github.com/repos/caskdata/cdap/subscribers","subscription_url":"https://api.github.com/repos/caskdata/cdap/subscription","commits_url":"https://api.github.com/repos/caskdata/cdap/commits{/sha}","git_commits_url":"https://api.github.com/repos/caskdata/cdap/git/commits{/sha}","comments_url":"https://api.github.com/repos/caskdata/cdap/comments{/number}","issue_comment_url":"https://api.github.com/repos/caskdata/cdap/issues/comments/{number}","contents_url":"https://api.github.com/repos/caskdata/cdap/contents/{+path}","compare_url":"https://api.github.com/repos/caskdata/cdap/compare/{base}...{head}","merges_url":"https://api.github.com/repos/caskdata/cdap/merges","archive_url":"https://api.github.com/repos/caskdata/cdap/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/caskdata/cdap/downloads","issues_url":"https://api.github.com/repos/caskdata/cdap/issues{/number}","pulls_url":"https://api.github.com/repos/caskdata/cdap/pulls{/number}","milestones_url":"https://api.github.com/repos/caskdata/cdap/milestones{/number}","notifications_url":"https://api.github.com/repos/caskdata/cdap/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/caskdata/cdap/labels{/name}","releases_url":"https://api.github.com/repos/caskdata/cdap/releases{/id}","created_at":"2014-08-02T09:00:10Z","updated_at":"2014-10-07T17:15:07Z","pushed_at":"2014-10-08T22:31:52Z","git_url":"git://github.com/caskdata/cdap.git","ssh_url":"git@github.com:caskdata/cdap.git","clone_url":"https://github.com/caskdata/cdap.git","svn_url":"https://github.com/caskdata/cdap","homepage":"http://cdap.io","size":380549,"stargazers_count":10,"watchers_count":10,"language":"Java","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":true,"forks_count":3,"mirror_url":null,"open_issues_count":9,"forks":3,"open_issues":9,"watchers":10,"default_branch":"develop"}},"_links":{"self":{"href":"https://api.github.com/repos/caskdata/cdap/pulls/510"},"html":{"href":"https://github.com/caskdata/cdap/pull/510"},"issue":{"href":"https://api.github.com/repos/caskdata/cdap/issues/510"},"comments":{"href":"https://api.github.com/repos/caskdata/cdap/issues/510/comments"},"review_comments":{"href":"https://api.github.com/repos/caskdata/cdap/pulls/510/comments"},"review_comment":{"href":"https://api.github.com/repos/caskdata/cdap/pulls/comments/{number}"},"commits":{"href":"https://api.github.com/repos/caskdata/cdap/pulls/510/commits"},"statuses":{"href":"https://api.github.com/repos/caskdata/cdap/statuses/3ee74b1b2b2dad174ebe4ff01ee5b3c690e48f7e"}}}}