You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by ahgittin <gi...@git.apache.org> on 2017/09/20 16:06:44 UTC

[GitHub] brooklyn-server pull request #836: REST call to get activities on an adjunct

GitHub user ahgittin opened a pull request:

    https://github.com/apache/brooklyn-server/pull/836

    REST call to get activities on an adjunct

    A single small commit to get via REST the activities related to an adjunct, and descendant tasks.
    
    This depends on #810, #816, and #818 -- where most of the work is done -- but once those are in the only changes should be those in 935c743d57bc3199d2b333a34b09ec9f0b4b71fc .

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ahgittin/brooklyn-server adjunct-activities-rest-api

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/brooklyn-server/pull/836.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #836
    
----
commit 9b337035992f77c7e156bba4d3b3ce35438ca180
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2017-09-07T15:43:45Z

    REST API for bundles, types, and subtypes
    
    also tweaks to other classes to make them more usable, eg VersionedName comparables and bundle removal giving an OsgiBundleInstallationResult
    
    test in BundleAndTypeAndSubtypeResourcesTest based on CatalogResourceTest, basically giving parity in terms of functionality and test coverage

commit 1cd6bf455d4cd0004a98881cc2d9a045bf2ffd1e
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2017-09-13T11:53:30Z

    Merge branch 'master' into bundle-rest-api
    
    BrooklynCatalog api tidy minor conflict

commit 80a4c255d595d4df9a2af7199b9a03e9f2a5641d
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2017-09-13T15:52:14Z

    Merge branch 'bundle-rest-api' into adjunct-rest-api-and-highlights-2

commit 2860af56a7e77de8742fb3b6a0c22903f33e3a6b
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2017-09-13T15:52:27Z

    Merge branch 'highlights-adjuncts' into adjunct-rest-api-and-highlights-2

commit 45261c7e155e1022cd172d9816a647ba35f0218c
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2017-09-13T15:22:56Z

    add adjunct REST API, deprecating policy endpoint

commit a11af34c33c69ef00adeb8906bb81d0776915c6b
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2017-09-08T15:00:29Z

    don't repeat task tracking code between scheduled and non-scheduled code paths

commit 438846c2967a64e109395a258effbf5b4db2cfd4
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2017-09-12T21:11:56Z

    optimization: skip tags copy on common simple lookups

commit 9d2faf0eddcd37ed0ec31cf1b9856a95b5702e14
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2017-09-11T07:40:31Z

    more bookkeeping for immediate/same-thread tasks

commit 4c2468d0f001bfdde74dd5b48c90935d0d746981
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2017-09-11T10:04:08Z

    mark task names needing tidy as TODO and tidy some

commit 784bc960f926823be374fa05d3df3fd542ee857a
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2017-09-11T17:00:11Z

    use new public class MaybeSupplier for Maybe instances backed by supplier
    
    so callers can access the supplier in cases where it might also provide other info such as Identifiable

commit 9a105e05dbe0d8019a7b4f3c3abd523e66a24a84
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2017-09-11T17:03:37Z

    remove soft refs in tasks in favour of lookup
    
    soft refs are expensive in the GC phase -- improves speed by 20% in some cases

commit 8983bf2cdae3e1b3bbc00d12cfb4baf09fb732fe
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2017-09-11T22:15:50Z

    add support for same-thread execution
    
    submitting and blocking is unnecessary overhead in many places

commit 934cf4ccbed5719f1b22ff032af77583c1cf6ad2
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2017-09-12T11:12:00Z

    diff task submisson paths share code about how to end, and tidy listeners
    
    fix bug where some where executed and GC'd before the child had actually run,
    by ensuring listeners don't run until the task recognizes the cancellation.
    
    also tidy how end code is shared, and remove deprecation

commit 18a908bd42f5a09da838c2f90bd6f79b5cd14766
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2017-09-12T16:31:42Z

    add and use builder for ScheduledTask

commit a341cceddb303d0076b9632b3fa64e3a7ffaa4f9
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2017-09-12T16:36:28Z

    remove interim TODO comments about checking that things work

commit c1f78bb2ba82504b2f0bfc1aa4831564d1f314b4
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2017-09-15T08:21:29Z

    tidy BasicExecutionManagement constructors

commit 5e19dec5180cc22066f91a4c229974e09a9d1ac1
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2017-09-15T09:52:15Z

    more testing of task cancel (in response to jenkins failure)

commit 7c2c8e1aa9a7a2c0be9476c93bd5f8c46c77e1ae
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2017-09-15T17:30:11Z

    improve task cancellation code
    
    consistently notify listeners and update maps and counts, and remove deprecated internal listeners class;
    add logging and speed increase to related tests (and Asserts.eventually can wait on on object to speed things up!)

commit 62359c60e54feebe0c38146d8eb6bfb055d8962a
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2017-09-16T01:54:25Z

    Merge branch 'master' into tasks-better
    
    conflict in adjacent line changes in Poller, fixed

commit 26dffef47d5d443b538d0b5a16dc133f01960372
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2017-09-15T16:43:58Z

    more tests for edge case where cancel doesn't cause listeners to run, and more tests for cancel

commit 24250a5e2c4e380b06e72fc9db52d3e5989fd00e
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2017-09-16T03:43:31Z

    preserve errors as much as possible when submitted to run in same thread

commit ab480ed288e7ccca90f8c05c06a817169cedd659
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2017-09-17T11:47:10Z

    tidy how cross-context submitted, and use simple tasks in more places

commit 2bdcf1ac81853410100920968f92a82c41b1f5b6
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2017-09-18T11:49:51Z

    apply context-switching task wrapper for same-thread and immediate tasks
    
    now behave same as submitted tasks. add tests, and comments about limits of getImmediately.

commit 37b6b11452b9619decde71376f95b73eab0836b4
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2017-09-12T10:49:51Z

    task visibility: validation of config

commit 7f4d7bd87e0e0e1d98ed49d875e601a21e0635c8
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2017-09-12T14:01:09Z

    task visibility: entity mgmt create and startup wrapped in its own task

commit 4430f769077210bf253a8d70a69482c1c2119d39
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2017-09-15T10:07:27Z

    task optimization: some queued-or-submitted tasks use foreground for executing

commit 8ecf3950ac1c158e6cac815c596ae0906439363b
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2017-09-15T10:24:43Z

    task visibility: better names for config retrieval tasks

commit 84d24d1a949275e6bad3bbabefe6bf422f9fade7
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2017-09-19T12:09:39Z

    fix visibility: entity init runs in same thread

commit d4c9fe12ecfdf884ea8e1945e51cc07e026d7610
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2017-09-19T12:10:17Z

    entity adjuncts have extra tag for execution context, used in subscription delivery

commit 0a1acecbe7b70ca752e0636605b34317f24a5a8e
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2017-09-19T12:09:59Z

    fix deadlock in initial publication of sensors on setting up a subscription

----


---

[GitHub] brooklyn-server pull request #836: REST call to get activities on an adjunct

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/836#discussion_r142926959
  
    --- Diff: rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/AdjunctApi.java ---
    @@ -234,4 +236,21 @@ public Response setConfig(
                 @PathParam("config") String configKeyName,
                 @ApiParam(name = "value", value = "New value for the configuration", required = true)
                 Object value);
    +    
    +
    +    @GET
    +    @Path("/{adjunct}/activities")
    +    @ApiOperation(value = "Fetch list of tasks for this adjunct")
    +    @ApiResponses(value = {
    +            @ApiResponse(code = 404, message = "Could not find application, entity, or adjunct")
    +    })
    +    public List<TaskSummary> listTasks(
    +            @ApiParam(value = "Application ID or name", required = true) @PathParam("application") String applicationId,
    +            @ApiParam(value = "Entity ID or name", required = true) @PathParam("entity") String entityId,
    +            @ApiParam(value = "Adjunct ID or name", required = true) @PathParam("adjunct") String adjunctToken,
    +            @ApiParam(value = "Max number of tasks, or -1 for all (default 200)", required = false) 
    --- End diff --
    
    I'm not a fan of `-1` for all. But it's already used in `EntityApi.listTasks` so I'll live with it.


---

[GitHub] brooklyn-server pull request #836: REST call to get activities on an adjunct

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/brooklyn-server/pull/836


---

[GitHub] brooklyn-server issue #836: REST call to get activities on an adjunct

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:

    https://github.com/apache/brooklyn-server/pull/836
  
    By the way, I don't entirely trust merge commits inside a pull request. It has seemed to me previously that `/files` in github has not shown me all the code changes because some are implicit in a merge commit. I'm not sure of the exact situation though. It happened when I was once trying to take a PR's changes into a maintenance branch, but it seemed that the `.patch` github mechanism didn't treat the merge commits as I'd have expected.
    
    I know that you have quite a sophisticated way of using git branches and merges locally, but when it reaches the PR stage is it not possible to just have it rebased against master such that it appears as a series of normal commits on top of brooklyn-server master?


---

[GitHub] brooklyn-server issue #836: REST call to get activities on an adjunct

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:

    https://github.com/apache/brooklyn-server/pull/836
  
    @aledsage the one thing i find with merges is that it's a bit difficult to see changes made to resolve conflicts as part of the merge commit.  i always forget the exact command but manage to find it. 
    
    however it keeps much better history, e.g.
    
    ```
    % git log --oneline --graph master..
    *   cea2d34 Merge branch 'tasks-better-tree' into adjunct-activities-rest-api
    |\  
    | * 9213f0e fix message publish synching to guarantee in-order delivery
    | * 508183b more assertion routines, map equality and unordered iterable equality
    | * 8b72769 Tasks.tryQueueing won't queue if calling thread is interrupted
    | * 9888870 switch queue-or-submit-blocking-then-get invocations to new simpler DynamicTasks.get. but note some things fail getImmediately if they are running a queueing context eg EffectorSayHiTest, until fixed in the next PR.
    * |   b042989 Merge branch 'tasks-better-tree' into adjunct-activities-rest-api
    |\ \  
    | |/  
    | * 0c2e1f6 Merge branch 'master' into tasks-4
    | * bb26d32 change when cancellation is done for getImmediate - means effector invocation now works
    | * d7b086b Merge branch 'tasks-better' into tasks-better-tree
    * | d1ab00c REST API for entity adjunct activities
    * | cf56492 Merge branch 'tasks-better-tree' into adj10
    |/  
    * 6dfe498 deprecated since is now 0.13.0 not 0.12.0
    * 80446ab Merge branch 'master' into tasks-better-tree
    * 130a29b fix tests that asserted specific tasks (as there are now more)
    * 79cc9bc task visibility: ensure all tasks have a name, updating exec.submit() methods to take name
    * aeecd3e task GC and visibility: tidy GC code, don't delete some things eg subscriptions quite so aggressively
    * b0556de include adjunct info as a subscription description
    * e1f948a task visibility: entity initialization
    * 0a1acec fix deadlock in initial publication of sensors on setting up a subscription
    * 84d24d1 fix visibility: entity init runs in same thread
    * d4c9fe1 entity adjuncts have extra tag for execution context, used in subscription delivery
    * 8ecf395 task visibility: better names for config retrieval tasks
    * 4430f76 task optimization: some queued-or-submitted tasks use foreground for executing
    * 7f4d7bd task visibility: entity mgmt create and startup wrapped in its own task
    ```
    
    it's usually prettier but even here not hard to see `d1ab00c` as the important mainline commit.
    
    more impressive is:
    
    ```
    % git log --oneline --graph tasks-better-tree..
    * cea2d34 Merge branch 'tasks-better-tree' into adjunct-activities-rest-api
    * b042989 Merge branch 'tasks-better-tree' into adjunct-activities-rest-api
    * d1ab00c REST API for entity adjunct activities
    * cf56492 Merge branch 'tasks-better-tree' into adj10
    ```
    
    making it super easy to see.  and bear in mind this at one point built on 3 other branches.  if i'd ever rebased this or any of those branches, the ability to reconcile changes would be gone.  with `git merge` however it was always straightforward.



---

[GitHub] brooklyn-server issue #836: REST call to get activities on an adjunct

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:

    https://github.com/apache/brooklyn-server/pull/836
  
    #821 now merged and #835 almost ready for review; this updated but still depends on #835
    
    still the only commit beyond that needing review here is 
    https://github.com/apache/brooklyn-server/commit/d1ab00c0f65c9444fd655bbf8759c982fe1d02ab


---

[GitHub] brooklyn-server pull request #836: REST call to get activities on an adjunct

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/836#discussion_r142982015
  
    --- Diff: rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/AdjunctApi.java ---
    @@ -234,4 +236,21 @@ public Response setConfig(
                 @PathParam("config") String configKeyName,
                 @ApiParam(name = "value", value = "New value for the configuration", required = true)
                 Object value);
    +    
    +
    +    @GET
    +    @Path("/{adjunct}/activities")
    +    @ApiOperation(value = "Fetch list of tasks for this adjunct")
    +    @ApiResponses(value = {
    +            @ApiResponse(code = 404, message = "Could not find application, entity, or adjunct")
    +    })
    +    public List<TaskSummary> listTasks(
    +            @ApiParam(value = "Application ID or name", required = true) @PathParam("application") String applicationId,
    +            @ApiParam(value = "Entity ID or name", required = true) @PathParam("entity") String entityId,
    +            @ApiParam(value = "Adjunct ID or name", required = true) @PathParam("adjunct") String adjunctToken,
    +            @ApiParam(value = "Max number of tasks, or -1 for all (default 200)", required = false) 
    --- End diff --
    
    yes for me it is simply "least bad"


---