[GitHub] qpid-dispatch pull request #393: DISPATCH-1143: New feature: connection-scop...

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] qpid-dispatch pull request #393: DISPATCH-1143: New feature: connection-scop...

asfgit
GitHub user kgiusti opened a pull request:

    https://github.com/apache/qpid-dispatch/pull/393

    DISPATCH-1143: New feature: connection-scoped link routes

    Haven't changed the name yet - still attachSubscription.   Will do that with a separate patch once the link route replication stuff is done.

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

    $ git pull https://github.com/kgiusti/dispatch DISPATCH-1143

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

    https://github.com/apache/qpid-dispatch/pull/393.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 #393
   
----
commit ead83d3a7ff9eb0c71624fd636e19cacacee6153
Author: Kenneth Giusti <kgiusti@...>
Date:   2018-10-08T20:06:45Z

    DISPATCH-1143: New feature: connection-scoped link routes

----


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] qpid-dispatch issue #393: DISPATCH-1143: New feature: connection-scoped link...

asfgit
Github user codecov-io commented on the issue:

    https://github.com/apache/qpid-dispatch/pull/393
 
    # [Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/393?src=pr&el=h1) Report
    > :exclamation: No coverage uploaded for pull request base (`master@7baa254`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
    > The diff coverage is `86.85%`.
   
    [![Impacted file tree graph](https://codecov.io/gh/apache/qpid-dispatch/pull/393/graphs/tree.svg?width=650&token=rk2Cgd27pP&height=150&src=pr)](https://codecov.io/gh/apache/qpid-dispatch/pull/393?src=pr&el=tree)
   
    ```diff
    @@            Coverage Diff            @@
    ##             master     #393   +/-   ##
    =========================================
      Coverage          ?   85.08%          
    =========================================
      Files             ?       77          
      Lines             ?    17028          
      Branches          ?        0          
    =========================================
      Hits              ?    14488          
      Misses            ?     2540          
      Partials          ?        0
    ```
   
   
    | [Impacted Files](https://codecov.io/gh/apache/qpid-dispatch/pull/393?src=pr&el=tree) | Coverage Δ | |
    |---|---|---|
    | [src/dispatch.c](https://codecov.io/gh/apache/qpid-dispatch/pull/393/diff?src=pr&el=tree#diff-c3JjL2Rpc3BhdGNoLmM=) | `86.51% <100%> (ø)` | |
    | [src/router\_core/forwarder.c](https://codecov.io/gh/apache/qpid-dispatch/pull/393/diff?src=pr&el=tree#diff-c3JjL3JvdXRlcl9jb3JlL2ZvcndhcmRlci5j) | `93.99% <100%> (ø)` | |
    | [src/router\_core/route\_control.c](https://codecov.io/gh/apache/qpid-dispatch/pull/393/diff?src=pr&el=tree#diff-c3JjL3JvdXRlcl9jb3JlL3JvdXRlX2NvbnRyb2wuYw==) | `96.47% <100%> (ø)` | |
    | [src/router\_core/agent\_config\_link\_route.c](https://codecov.io/gh/apache/qpid-dispatch/pull/393/diff?src=pr&el=tree#diff-c3JjL3JvdXRlcl9jb3JlL2FnZW50X2NvbmZpZ19saW5rX3JvdXRlLmM=) | `86.74% <100%> (ø)` | |
    | [src/python\_embedded.c](https://codecov.io/gh/apache/qpid-dispatch/pull/393/diff?src=pr&el=tree#diff-c3JjL3B5dGhvbl9lbWJlZGRlZC5j) | `77.59% <100%> (ø)` | |
    | [src/router\_config.c](https://codecov.io/gh/apache/qpid-dispatch/pull/393/diff?src=pr&el=tree#diff-c3JjL3JvdXRlcl9jb25maWcuYw==) | `96.37% <100%> (ø)` | |
    | [src/router\_core/router\_core.c](https://codecov.io/gh/apache/qpid-dispatch/pull/393/diff?src=pr&el=tree#diff-c3JjL3JvdXRlcl9jb3JlL3JvdXRlcl9jb3JlLmM=) | `93.33% <100%> (ø)` | |
    | [src/router\_core/agent\_attach\_subscription.c](https://codecov.io/gh/apache/qpid-dispatch/pull/393/diff?src=pr&el=tree#diff-c3JjL3JvdXRlcl9jb3JlL2FnZW50X2F0dGFjaF9zdWJzY3JpcHRpb24uYw==) | `81.87% <81.87%> (ø)` | |
    | [src/router\_core/management\_agent.c](https://codecov.io/gh/apache/qpid-dispatch/pull/393/diff?src=pr&el=tree#diff-c3JjL3JvdXRlcl9jb3JlL21hbmFnZW1lbnRfYWdlbnQuYw==) | `92.61% <84.61%> (ø)` | |
    | [src/router\_core/agent.c](https://codecov.io/gh/apache/qpid-dispatch/pull/393/diff?src=pr&el=tree#diff-c3JjL3JvdXRlcl9jb3JlL2FnZW50LmM=) | `78.9% <86.66%> (ø)` | |
   
    ------
   
    [Continue to review full report at Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/393?src=pr&el=continue).
    > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
    > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
    > Powered by [Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/393?src=pr&el=footer). Last update [7baa254...ead83d3](https://codecov.io/gh/apache/qpid-dispatch/pull/393?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).



---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] qpid-dispatch pull request #393: DISPATCH-1143: New feature: connection-scop...

asfgit
In reply to this post by asfgit
Github user ted-ross commented on a diff in the pull request:

    https://github.com/apache/qpid-dispatch/pull/393#discussion_r226733939
 
    --- Diff: python/qpid_dispatch_internal/management/config.py ---
    @@ -67,6 +67,7 @@ def transform_sections(sections):
                 if s[0] == "autoLink":  s[0] = "router.config.autoLink"
                 if s[0] == "exchange":  s[0] = "router.config.exchange"
                 if s[0] == "binding":   s[0] = "router.config.binding"
    +            if s[0] == "attachSubscription":   s[0] = "router.connection.attachSubscription"
    --- End diff --
   
    Why go to all the trouble of adding support for configuration of a non-configurable entity?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] qpid-dispatch pull request #393: DISPATCH-1143: New feature: connection-scop...

asfgit
In reply to this post by asfgit
Github user ted-ross commented on a diff in the pull request:

    https://github.com/apache/qpid-dispatch/pull/393#discussion_r226733245
 
    --- Diff: include/qpid/dispatch/router_core.h ---
    @@ -721,10 +723,11 @@ typedef struct qdr_query_t qdr_query_t;
      * @param name The name supplied for the entity
      * @param in_body The body of the request message
      * @param out_body A composed field for the body of the response message
    + * @param in_conn The identity of the connection over which the mgmt message arrived (0 if config file)
      */
     void qdr_manage_create(qdr_core_t *core, void *context, qd_router_entity_type_t type,
                            qd_iterator_t *name, qd_parsed_field_t *in_body, qd_composed_field_t *out_body,
    -                       qd_buffer_list_t body_buffers);
    +                       qd_buffer_list_t body_buffers, uint64_t in_conn);
    --- End diff --
   
    Is it possible to use the qdr_connection_t* here instead of an ID that's going to have to be searched for later?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] qpid-dispatch pull request #393: DISPATCH-1143: New feature: connection-scop...

asfgit
In reply to this post by asfgit
Github user ted-ross commented on a diff in the pull request:

    https://github.com/apache/qpid-dispatch/pull/393#discussion_r226733581
 
    --- Diff: python/qpid_dispatch_internal/management/agent.py ---
    @@ -417,6 +417,13 @@ def create(self):
         def __str__(self):
             return super(LinkRouteEntity, self).__str__().replace("Entity(", "LinkRouteEntity(")
     
    +class RouterConnectionAttachsubscriptionEntity(EntityAdapter):
    --- End diff --
   
    Why is this even mentioned in the Python management code?  It can't be configured and management commands are handled in the router core.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] qpid-dispatch pull request #393: DISPATCH-1143: New feature: connection-scop...

asfgit
In reply to this post by asfgit
Github user kgiusti commented on a diff in the pull request:

    https://github.com/apache/qpid-dispatch/pull/393#discussion_r226742200
 
    --- Diff: python/qpid_dispatch_internal/management/agent.py ---
    @@ -417,6 +417,13 @@ def create(self):
         def __str__(self):
             return super(LinkRouteEntity, self).__str__().replace("Entity(", "LinkRouteEntity(")
     
    +class RouterConnectionAttachsubscriptionEntity(EntityAdapter):
    --- End diff --
   
    Error reporting.  Without it the router fails with a generic parse error. With this we get:
   
    qdrouterd: Python: CError: Configuration: attachSubscription cannot be created via the configuration file
   
    I'm going to have to re-work this on changing the name in any case.  I can pull it if you want.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] qpid-dispatch pull request #393: DISPATCH-1143: New feature: connection-scop...

asfgit
In reply to this post by asfgit
Github user kgiusti commented on a diff in the pull request:

    https://github.com/apache/qpid-dispatch/pull/393#discussion_r226763750
 
    --- Diff: python/qpid_dispatch_internal/management/agent.py ---
    @@ -417,6 +417,13 @@ def create(self):
         def __str__(self):
             return super(LinkRouteEntity, self).__str__().replace("Entity(", "LinkRouteEntity(")
     
    +class RouterConnectionAttachsubscriptionEntity(EntityAdapter):
    --- End diff --
   
    Ya know, the default error message isn't too bad:
   
    2018-10-19 15:37:07.199514 -0400 ERROR (error) Python: Exception: Cannot load configuration file X.conf: No such entity type 'org.apache.qpid.dispatch.attachSubscription'
    2018-10-19 15:37:07.199789 -0400 ERROR (error) Traceback (most recent call last):
      File "/home/kgiusti/work/dispatch/qpid-dispatch/python/qpid_dispatch_internal/management/config.py", line 169, in configure_dispatch
        config = Config(filename)
      File "/home/kgiusti/work/dispatch/qpid-dispatch/python/qpid_dispatch_internal/management/config.py", line 56, in __init__
        % (filename, e))
    Exception: Cannot load configuration file X.conf: No such entity type 'org.apache.qpid.dispatch.attachSubscription'
   
    2018-10-19 15:37:07.199805 -0400 MAIN (critical) Router start-up failed: Python: Exception: Cannot load configuration file X.conf: No such entity type 'org.apache.qpid.dispatch.attachSubscription'
    qdrouterd: Python: Exception: Cannot load configuration file X.conf: No such entity type 'org.apache.qpid.dispatch.attachSubscription'
   
    A little better than a generic parse error.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] qpid-dispatch pull request #393: DISPATCH-1143: New feature: connection-scop...

asfgit
In reply to this post by asfgit
Github user ted-ross commented on a diff in the pull request:

    https://github.com/apache/qpid-dispatch/pull/393#discussion_r227029464
 
    --- Diff: include/qpid/dispatch/router_core.h ---
    @@ -721,10 +723,11 @@ typedef struct qdr_query_t qdr_query_t;
      * @param name The name supplied for the entity
      * @param in_body The body of the request message
      * @param out_body A composed field for the body of the response message
    + * @param in_conn The identity of the connection over which the mgmt message arrived (0 if config file)
      */
     void qdr_manage_create(qdr_core_t *core, void *context, qd_router_entity_type_t type,
                            qd_iterator_t *name, qd_parsed_field_t *in_body, qd_composed_field_t *out_body,
    -                       qd_buffer_list_t body_buffers);
    +                       qd_buffer_list_t body_buffers, uint64_t in_conn);
    --- End diff --
   
    Yes, I think that is possible.  I'd like to give this some more thought.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]