GitHub PullRequestReviewCommentEvent at 2014-07-29 12:23:47
Repository: incubator-brooklyn
Actor: aledsage
Payload: {"action":"created","comment":{"url":"https://api.github.com/repos/apache/incubator-brooklyn/pulls/comments/15519562","id":15519562,"diff_hunk":"@@ -127,4 +158,140 @@ protected boolean calculateServiceUp() {\n         }\n         return up;\n     }\n+    \n+    // TODO this will probably be useful elsewhere ... but where to put it?\n+    /** Waits for the given target to report service up, then runs the given task\n+     * (often an invocation on that entity), with the given name.\n+     * If the target goes away, this task marks itself inessential\n+     * before failing so as not to cause a parent task to fail. */\n+    static <T> Task<T> whenServiceUp(final Entity target, final TaskAdaptable<T> task, String name) {\n+        return Tasks.<T>builder().name(name).dynamic(true).body(new Callable<T>() {\n+            @Override\n+            public T call() {\n+                try {\n+                    while (true) {\n+                        if (!Entities.isManaged(target)) {\n+                            Tasks.markInessential();\n+                            throw new IllegalStateException(\"Target \"+target+\" is no longer managed\");\n+                        }\n+                        if (target.getAttribute(Attributes.SERVICE_UP)) {\n+                            Tasks.resetBlockingDetails();\n+                            TaskTags.markInessential(task);\n+                            DynamicTasks.queue(task);\n+                            try {\n+                                return task.asTask().getUnchecked();\n+                            } catch (Exception e) {\n+                                if (Entities.isManaged(target)) {\n+                                    throw Exceptions.propagate(e);\n+                                } else {\n+                                    Tasks.markInessential();\n+                                    throw new IllegalStateException(\"Target \"+target+\" is no longer managed\", e);\n+                                }\n+                            }\n+                        } else {\n+                            Tasks.setBlockingDetails(\"Waiting on \"+target+\" to be ready\");\n+                        }\n+                        // TODO replace with subscription?\n+                        Time.sleep(Duration.ONE_SECOND);\n+                    }\n+                } finally {\n+                    Tasks.resetBlockingDetails();\n+                }\n+            }\n+        }).build();        \n+    }\n+\n+    @Override\n+    @Effector(description=\"Deploys the given artifact, from a source URL, to a given deployment filename/context\")\n+    public void deploy(\n+            @EffectorParam(name=\"url\", description=\"URL of WAR file\") String url, \n+            @EffectorParam(name=\"targetName\", description=\"context path where WAR should be deployed (/ for ROOT)\") String targetName) {\n+        checkNotNull(url, \"url\");\n+        checkNotNull(targetName, \"targetName\");\n+        targetName = filenameToWebContextMapper.convertDeploymentTargetNameToContext(targetName);\n+\n+        // set it up so future nodes get the right wars\n+        addToWarsByContext(this, url, targetName);\n+        \n+        log.debug(\"Deploying \"+targetName+\"->\"+url+\" across cluster \"+this+\"; WARs now \"+getConfig(WARS_BY_CONTEXT));\n+\n+        Iterable<CanDeployAndUndeploy> targets = Iterables.filter(getChildren(), CanDeployAndUndeploy.class);\n+        TaskBuilder<Void> tb = Tasks.<Void>builder().parallel(true).name(\"Deploy \"+targetName+\" to cluster (size \"+Iterables.size(targets)+\")\");\n+        for (Entity target: targets) {\n+            tb.add(whenServiceUp(target, Effectors.invocation(target, DEPLOY, MutableMap.of(\"url\", url, \"targetName\", targetName)),\n+                \"Deploy \"+targetName+\" to \"+target+\" when ready\"));\n+        }\n+        DynamicTasks.queueIfPossible(tb.build()).orSubmitAsync(this).asTask().getUnchecked();\n+\n+        // Update attribute\n+        Set<String> deployedWars = MutableSet.copyOf(getAttribute(DEPLOYED_WARS));\n+        deployedWars.add(targetName);\n+        setAttribute(DEPLOYED_WARS, deployedWars);\n+    }\n+    \n+    @Override\n+    @Effector(description=\"Undeploys the given context/artifact\")\n+    public void undeploy(@EffectorParam(name=\"targetName\") String targetName) {\n+        checkNotNull(targetName, \"targetName\");\n+        targetName = filenameToWebContextMapper.convertDeploymentTargetNameToContext(targetName);\n+        \n+        // set it up so future nodes get the right wars\n+        removeFromWarsByContext(this, targetName);\n+        \n+        log.debug(\"Undeploying \"+targetName+\" across cluster \"+this+\"; WARs now \"+getConfig(WARS_BY_CONTEXT));\n+\n+        Iterable<CanDeployAndUndeploy> targets = Iterables.filter(getChildren(), CanDeployAndUndeploy.class);\n+        TaskBuilder<Void> tb = Tasks.<Void>builder().parallel(true).name(\"Undeploy \"+targetName+\" across cluster (size \"+Iterables.size(targets)+\")\");\n+        for (Entity target: targets) {\n+            tb.add(whenServiceUp(target, Effectors.invocation(target, UNDEPLOY, MutableMap.of(\"targetName\", targetName)),\n+                \"Undeploy \"+targetName+\" at \"+target+\" when ready\"));\n+        }\n+        DynamicTasks.queueIfPossible(tb.build()).orSubmitAsync(this).asTask().getUnchecked();\n+\n+        // Update attribute\n+        Set<String> deployedWars = MutableSet.copyOf(getAttribute(DEPLOYED_WARS));\n+        deployedWars.remove( filenameToWebContextMapper.convertDeploymentTargetNameToContext(targetName) );\n+        setAttribute(DEPLOYED_WARS, deployedWars);\n+    }\n+\n+    static void addToWarsByContext(Entity entity, String url, String targetName) {\n+        targetName = filenameToWebContextMapper.convertDeploymentTargetNameToContext(targetName);\n+        synchronized (entity) {\n+            Map<String,String> newWarsMap = MutableMap.copyOf(entity.getConfig(WARS_BY_CONTEXT));\n+            newWarsMap.put(targetName, url);\n+            ((EntityInternal)entity).setConfig(WARS_BY_CONTEXT, newWarsMap);\n+        }\n+    }\n+\n+    static void removeFromWarsByContext(Entity entity, String targetName) {\n+        targetName = filenameToWebContextMapper.convertDeploymentTargetNameToContext(targetName);\n+        synchronized (entity) {\n+            Map<String,String> newWarsMap = MutableMap.copyOf(entity.getConfig(WARS_BY_CONTEXT));\n+            String url = newWarsMap.remove(targetName);\n+            if (url==null) {\n+                DynamicTasks.submit(Tasks.warning(\"Context \"+targetName+\" not known at \"+entity+\"; attempting to undeploy regardless\", null), entity);","path":"software/webapp/src/main/java/brooklyn/entity/webapp/DynamicWebAppClusterImpl.java","position":179,"original_position":179,"commit_id":"436a0dd286110f072d7348abad9185b205afd5aa","original_commit_id":"436a0dd286110f072d7348abad9185b205afd5aa","user":{"login":"aledsage","id":593113,"avatar_url":"https://avatars.githubusercontent.com/u/593113?","gravatar_id":"e3028779c14cdec6ce964238c999d638","url":"https://api.github.com/users/aledsage","html_url":"https://github.com/aledsage","followers_url":"https://api.github.com/users/aledsage/followers","following_url":"https://api.github.com/users/aledsage/following{/other_user}","gists_url":"https://api.github.com/users/aledsage/gists{/gist_id}","starred_url":"https://api.github.com/users/aledsage/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/aledsage/subscriptions","organizations_url":"https://api.github.com/users/aledsage/orgs","repos_url":"https://api.github.com/users/aledsage/repos","events_url":"https://api.github.com/users/aledsage/events{/privacy}","received_events_url":"https://api.github.com/users/aledsage/received_events","type":"User","site_admin":false},"body":"Instead of warning here, I'd prefer this method to return true/false to say whether it was removed. If it wasn't then the caller can do the logging. The log message does not relate to what this method is doing.\r\n\r\nThe call in `ControlledDynamicWebAppClusterImpl` presumably doesn't need to do this logging as we'll get exactly the same check when that calls `getCluster().undeploy(target)`.","created_at":"2014-07-29T12:23:47Z","updated_at":"2014-07-29T12:23:47Z","html_url":"https://github.com/apache/incubator-brooklyn/pull/83#discussion_r15519562","pull_request_url":"https://api.github.com/repos/apache/incubator-brooklyn/pulls/83","_links":{"self":{"href":"https://api.github.com/repos/apache/incubator-brooklyn/pulls/comments/15519562"},"html":{"href":"https://github.com/apache/incubator-brooklyn/pull/83#discussion_r15519562"},"pull_request":{"href":"https://api.github.com/repos/apache/incubator-brooklyn/pulls/83"}}},"pull_request":{"url":"https://api.github.com/repos/apache/incubator-brooklyn/pulls/83","id":18696608,"html_url":"https://github.com/apache/incubator-brooklyn/pull/83","diff_url":"https://github.com/apache/incubator-brooklyn/pull/83.diff","patch_url":"https://github.com/apache/incubator-brooklyn/pull/83.patch","issue_url":"https://api.github.com/repos/apache/incubator-brooklyn/issues/83","number":83,"state":"open","title":"Deploy across an entire web cluster","user":{"login":"ahgittin","id":496540,"avatar_url":"https://avatars.githubusercontent.com/u/496540?","gravatar_id":"b2cc5103505bf11a36c99c961929db5b","url":"https://api.github.com/users/ahgittin","html_url":"https://github.com/ahgittin","followers_url":"https://api.github.com/users/ahgittin/followers","following_url":"https://api.github.com/users/ahgittin/following{/other_user}","gists_url":"https://api.github.com/users/ahgittin/gists{/gist_id}","starred_url":"https://api.github.com/users/ahgittin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ahgittin/subscriptions","organizations_url":"https://api.github.com/users/ahgittin/orgs","repos_url":"https://api.github.com/users/ahgittin/repos","events_url":"https://api.github.com/users/ahgittin/events{/privacy}","received_events_url":"https://api.github.com/users/ahgittin/received_events","type":"User","site_admin":false},"body":"Adds deploy, undeploy, and redeployAll to webapp cluster entities (DWAC and ControlledDWAC).\r\n\r\nAlso fixes some issues noticed in how tasks are tracked and displayed.","created_at":"2014-07-22T06:22:11Z","updated_at":"2014-07-29T12:23:47Z","closed_at":null,"merged_at":null,"merge_commit_sha":"307c59e5e914fa3bbb9b96c4f44ecf57a67fcbd0","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/apache/incubator-brooklyn/pulls/83/commits","review_comments_url":"https://api.github.com/repos/apache/incubator-brooklyn/pulls/83/comments","review_comment_url":"https://api.github.com/repos/apache/incubator-brooklyn/pulls/comments/{number}","comments_url":"https://api.github.com/repos/apache/incubator-brooklyn/issues/83/comments","statuses_url":"https://api.github.com/repos/apache/incubator-brooklyn/statuses/436a0dd286110f072d7348abad9185b205afd5aa","head":{"label":"ahgittin:web-cluster-deploy","ref":"web-cluster-deploy","sha":"436a0dd286110f072d7348abad9185b205afd5aa","user":{"login":"ahgittin","id":496540,"avatar_url":"https://avatars.githubusercontent.com/u/496540?","gravatar_id":"b2cc5103505bf11a36c99c961929db5b","url":"https://api.github.com/users/ahgittin","html_url":"https://github.com/ahgittin","followers_url":"https://api.github.com/users/ahgittin/followers","following_url":"https://api.github.com/users/ahgittin/following{/other_user}","gists_url":"https://api.github.com/users/ahgittin/gists{/gist_id}","starred_url":"https://api.github.com/users/ahgittin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ahgittin/subscriptions","organizations_url":"https://api.github.com/users/ahgittin/orgs","repos_url":"https://api.github.com/users/ahgittin/repos","events_url":"https://api.github.com/users/ahgittin/events{/privacy}","received_events_url":"https://api.github.com/users/ahgittin/received_events","type":"User","site_admin":false},"repo":{"id":21164564,"name":"incubator-brooklyn","full_name":"ahgittin/incubator-brooklyn","owner":{"login":"ahgittin","id":496540,"avatar_url":"https://avatars.githubusercontent.com/u/496540?","gravatar_id":"b2cc5103505bf11a36c99c961929db5b","url":"https://api.github.com/users/ahgittin","html_url":"https://github.com/ahgittin","followers_url":"https://api.github.com/users/ahgittin/followers","following_url":"https://api.github.com/users/ahgittin/following{/other_user}","gists_url":"https://api.github.com/users/ahgittin/gists{/gist_id}","starred_url":"https://api.github.com/users/ahgittin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ahgittin/subscriptions","organizations_url":"https://api.github.com/users/ahgittin/orgs","repos_url":"https://api.github.com/users/ahgittin/repos","events_url":"https://api.github.com/users/ahgittin/events{/privacy}","received_events_url":"https://api.github.com/users/ahgittin/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/ahgittin/incubator-brooklyn","description":"Mirror of Apache Brooklyn","fork":true,"url":"https://api.github.com/repos/ahgittin/incubator-brooklyn","forks_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/forks","keys_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/keys{/key_id}","collaborators_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/teams","hooks_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/hooks","issue_events_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/issues/events{/number}","events_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/events","assignees_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/assignees{/user}","branches_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/branches{/branch}","tags_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/tags","blobs_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/git/refs{/sha}","trees_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/git/trees{/sha}","statuses_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/statuses/{sha}","languages_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/languages","stargazers_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/stargazers","contributors_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/contributors","subscribers_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/subscribers","subscription_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/subscription","commits_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/commits{/sha}","git_commits_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/git/commits{/sha}","comments_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/comments{/number}","issue_comment_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/issues/comments/{number}","contents_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/contents/{+path}","compare_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/compare/{base}...{head}","merges_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/merges","archive_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/downloads","issues_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/issues{/number}","pulls_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/pulls{/number}","milestones_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/milestones{/number}","notifications_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/labels{/name}","releases_url":"https://api.github.com/repos/ahgittin/incubator-brooklyn/releases{/id}","created_at":"2014-06-24T12:59:32Z","updated_at":"2014-06-26T11:39:42Z","pushed_at":"2014-07-25T18:06:16Z","git_url":"git://github.com/ahgittin/incubator-brooklyn.git","ssh_url":"git@github.com:ahgittin/incubator-brooklyn.git","clone_url":"https://github.com/ahgittin/incubator-brooklyn.git","svn_url":"https://github.com/ahgittin/incubator-brooklyn","homepage":null,"size":225456,"stargazers_count":0,"watchers_count":0,"language":"Java","has_issues":false,"has_downloads":true,"has_wiki":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":"35f5ff2dbe8614915ae1df2bb72c730d26bfa74c","user":{"login":"apache","id":47359,"avatar_url":"https://avatars.githubusercontent.com/u/47359?","gravatar_id":"a676c0bf448fcd49f588249ead719b4c","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":19368221,"name":"incubator-brooklyn","full_name":"apache/incubator-brooklyn","owner":{"login":"apache","id":47359,"avatar_url":"https://avatars.githubusercontent.com/u/47359?","gravatar_id":"a676c0bf448fcd49f588249ead719b4c","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/incubator-brooklyn","description":"Mirror of Apache Brooklyn","fork":false,"url":"https://api.github.com/repos/apache/incubator-brooklyn","forks_url":"https://api.github.com/repos/apache/incubator-brooklyn/forks","keys_url":"https://api.github.com/repos/apache/incubator-brooklyn/keys{/key_id}","collaborators_url":"https://api.github.com/repos/apache/incubator-brooklyn/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/apache/incubator-brooklyn/teams","hooks_url":"https://api.github.com/repos/apache/incubator-brooklyn/hooks","issue_events_url":"https://api.github.com/repos/apache/incubator-brooklyn/issues/events{/number}","events_url":"https://api.github.com/repos/apache/incubator-brooklyn/events","assignees_url":"https://api.github.com/repos/apache/incubator-brooklyn/assignees{/user}","branches_url":"https://api.github.com/repos/apache/incubator-brooklyn/branches{/branch}","tags_url":"https://api.github.com/repos/apache/incubator-brooklyn/tags","blobs_url":"https://api.github.com/repos/apache/incubator-brooklyn/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/apache/incubator-brooklyn/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/apache/incubator-brooklyn/git/refs{/sha}","trees_url":"https://api.github.com/repos/apache/incubator-brooklyn/git/trees{/sha}","statuses_url":"https://api.github.com/repos/apache/incubator-brooklyn/statuses/{sha}","languages_url":"https://api.github.com/repos/apache/incubator-brooklyn/languages","stargazers_url":"https://api.github.com/repos/apache/incubator-brooklyn/stargazers","contributors_url":"https://api.github.com/repos/apache/incubator-brooklyn/contributors","subscribers_url":"https://api.github.com/repos/apache/incubator-brooklyn/subscribers","subscription_url":"https://api.github.com/repos/apache/incubator-brooklyn/subscription","commits_url":"https://api.github.com/repos/apache/incubator-brooklyn/commits{/sha}","git_commits_url":"https://api.github.com/repos/apache/incubator-brooklyn/git/commits{/sha}","comments_url":"https://api.github.com/repos/apache/incubator-brooklyn/comments{/number}","issue_comment_url":"https://api.github.com/repos/apache/incubator-brooklyn/issues/comments/{number}","contents_url":"https://api.github.com/repos/apache/incubator-brooklyn/contents/{+path}","compare_url":"https://api.github.com/repos/apache/incubator-brooklyn/compare/{base}...{head}","merges_url":"https://api.github.com/repos/apache/incubator-brooklyn/merges","archive_url":"https://api.github.com/repos/apache/incubator-brooklyn/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/apache/incubator-brooklyn/downloads","issues_url":"https://api.github.com/repos/apache/incubator-brooklyn/issues{/number}","pulls_url":"https://api.github.com/repos/apache/incubator-brooklyn/pulls{/number}","milestones_url":"https://api.github.com/repos/apache/incubator-brooklyn/milestones{/number}","notifications_url":"https://api.github.com/repos/apache/incubator-brooklyn/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/apache/incubator-brooklyn/labels{/name}","releases_url":"https://api.github.com/repos/apache/incubator-brooklyn/releases{/id}","created_at":"2014-05-02T07:00:07Z","updated_at":"2014-07-05T05:45:39Z","pushed_at":"2014-07-28T08:41:31Z","git_url":"git://github.com/apache/incubator-brooklyn.git","ssh_url":"git@github.com:apache/incubator-brooklyn.git","clone_url":"https://github.com/apache/incubator-brooklyn.git","svn_url":"https://github.com/apache/incubator-brooklyn","homepage":null,"size":230129,"stargazers_count":10,"watchers_count":10,"language":"Java","has_issues":false,"has_downloads":true,"has_wiki":false,"forks_count":18,"mirror_url":"git://git.apache.org/incubator-brooklyn.git","open_issues_count":18,"forks":18,"open_issues":18,"watchers":10,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/apache/incubator-brooklyn/pulls/83"},"html":{"href":"https://github.com/apache/incubator-brooklyn/pull/83"},"issue":{"href":"https://api.github.com/repos/apache/incubator-brooklyn/issues/83"},"comments":{"href":"https://api.github.com/repos/apache/incubator-brooklyn/issues/83/comments"},"review_comments":{"href":"https://api.github.com/repos/apache/incubator-brooklyn/pulls/83/comments"},"review_comment":{"href":"https://api.github.com/repos/apache/incubator-brooklyn/pulls/comments/{number}"},"commits":{"href":"https://api.github.com/repos/apache/incubator-brooklyn/pulls/83/commits"},"statuses":{"href":"https://api.github.com/repos/apache/incubator-brooklyn/statuses/436a0dd286110f072d7348abad9185b205afd5aa"}}}}