[GitHub] activemq-artemis pull request #1743: ARTEMIS-1575 configure custom broker pl...

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

[GitHub] activemq-artemis pull request #1743: ARTEMIS-1575 configure custom broker pl...

gaohoward-3
GitHub user pgfox opened a pull request:

    https://github.com/apache/activemq-artemis/pull/1743

    ARTEMIS-1575 configure custom broker plugins with key/value plugin-property(s).

   
    The plugin-property(s) are read and passed into the the broker plugin's init(Map<String,String>)

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

    $ git pull https://github.com/pgfox/activemq-artemis plugin_properties

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

    https://github.com/apache/activemq-artemis/pull/1743.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 #1743
   
----
commit 3e64fb4b02366a3e1ac9132f641d57fb1e2cf13e
Author: Pat Fox <pat.g.fox@...>
Date:   2017-12-23T20:04:31Z

    ARTEMIS-1575 allow users to configure custom broker plugins with key/value plugin-property(s).
   
    The plugin-property(s) are read and passed into the the broker plugin's init(Map<String,String>)

----


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1743: ARTEMIS-1575 configure custom broker pl...

gaohoward-3
Github user michaelandrepearce commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1743#discussion_r159060268
 
    --- Diff: artemis-server/src/main/resources/schema/artemis-configuration.xsd ---
    @@ -934,6 +934,26 @@
                                   a broker plugin
                                </xsd:documentation>
                             </xsd:annotation>
    +                        <xsd:sequence>
    --- End diff --
   
    Is it worth instead of re-defineing what a property type is, re-naming and re-using the transformerProperty complex type which this is identical to?
   
    also could the same file  /artemis-tools/src/test/resources/artemis-configuration.xsd be updated also.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1743: ARTEMIS-1575 configure custom broker pl...

gaohoward-3
In reply to this post by gaohoward-3
Github user pgfox commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1743#discussion_r159075657
 
    --- Diff: artemis-server/src/main/resources/schema/artemis-configuration.xsd ---
    @@ -934,6 +934,26 @@
                                   a broker plugin
                                </xsd:documentation>
                             </xsd:annotation>
    +                        <xsd:sequence>
    --- End diff --
   
    Sure, I was originally hesitant to mess with your transformerProperty but happy to do so if it is ok with you.  I will make the changes you suggested next week,
   
    Thanks.
     


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1743: ARTEMIS-1575 configure custom broker plugins w...

gaohoward-3
In reply to this post by gaohoward-3
Github user pgfox commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1743
 
    @michaelandrepearce  suggested changes made.
   
    I defined an element "property" around the complex type and reused it in both places.  The element definition baked in the naming convention so the parsing code was a little easier/robust.
   
    I updated the bit that where already in /artemis-tools/src/test/resources/artemis-configuration.xsd .
    There seems to be some structure missing from that file. Not sure if it is an oversight or intended.
   
   



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1743: ARTEMIS-1575 configure custom broker plugins w...

gaohoward-3
In reply to this post by gaohoward-3
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1743
 
    @pgfox great stuff! LGTM, will look to merge for you tonight/tomorrow, pending no further comments from anyone else. Thanks for the contribution.
   
    re the tools schema, yeah i noticed it too when I've done work. Tbh i think longer term i wonder if we can ditch the files there and simply the build copies them in or something to avoid the duplication. I don't know the history on it though unfortunately maybe someone else knows better.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1743: ARTEMIS-1575 configure custom broker pl...

gaohoward-3
In reply to this post by gaohoward-3
Github user asfgit closed the pull request at:

    https://github.com/apache/activemq-artemis/pull/1743


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1743: ARTEMIS-1575 configure custom broker plugins w...

gaohoward-3
In reply to this post by gaohoward-3
Github user pgfox commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1743
 
    @michaelandrepearce  Thanks


---