You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2021/05/03 23:13:42 UTC
[GitHub] [activemq-artemis] ehossack-aws opened a new pull request #3564: NO-JIRA remove ordering constraint from tag in XML
ehossack-aws opened a new pull request #3564:
URL: https://github.com/apache/activemq-artemis/pull/3564
It seems like the XSD requires the elements of the `<broadcast-group>` tags to be in order. Based on my investigation, I cannot see a reason for this, as it has been so since the HornetQ donation. `<discovery-group>` [is not ordered](https://github.com/apache/activemq-artemis/blob/f491651fdbc911e11c470afb3c0f7f6da1d74063/artemis-server/src/main/resources/schema/artemis-configuration.xsd#L1371).
Without this change, users may encounter cryptic error messages such as the below if they apply configuration [as in the documentation](https://github.com/apache/activemq-artemis/blame/f491651fdbc911e11c470afb3c0f7f6da1d74063/docs/user-manual/en/clusters.md#L153)
```
java.lang.IllegalStateException: Invalid configuration
at org.apache.activemq.artemis.utils.XMLUtil.validate(XMLUtil.java:365)
at org.apache.activemq.artemis.core.config.FileDeploymentManager.readConfiguration(FileDeploymentManager.java:80)
at org.apache.activemq.artemis.integration.FileBroker.createComponents(FileBroker.java:118)
at org.apache.activemq.artemis.cli.commands.Run.execute(Run.java:112)
at org.apache.activemq.artemis.cli.Artemis.internalExecute(Artemis.java:154)
at org.apache.activemq.artemis.cli.Artemis.execute(Artemis.java:102)
at org.apache.activemq.artemis.cli.Artemis.execute(Artemis.java:129)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
at java.base/java.lang.reflect.Method.invoke(Unknown Source)
at org.apache.activemq.artemis.boot.Artemis.execute(Artemis.java:134)
at org.apache.activemq.artemis.boot.Artemis.main(Artemis.java:50)
Caused by: org.xml.sax.SAXParseException; cvc-complex-type.2.4.a: Invalid content was found starting with element '{"urn:activemq:core":broadcast-period}'. One of '{"urn:activemq:core":connector-ref}' is expected.
at java.xml/com.sun.org.apache.xerces.internal.util.ErrorHandlerWrapper.createSAXParseException(Unknown Source)
at java.xml/com.sun.org.apache.xerces.internal.util.ErrorHandlerWrapper.error(Unknown Source)
at java.xml/com.sun.org.apache.xerces.internal.impl.XMLErrorReporter.reportError(Unknown Source)
at java.xml/com.sun.org.apache.xerces.internal.impl.XMLErrorReporter.reportError(Unknown Source)
at java.xml/com.sun.org.apache.xerces.internal.impl.XMLErrorReporter.reportError(Unknown Source)
at java.xml/com.sun.org.apache.xerces.internal.impl.xs.XMLSchemaValidator$XSIErrorReporter.reportError(Unknown Source)
at java.xml/com.sun.org.apache.xerces.internal.impl.xs.XMLSchemaValidator.reportSchemaError(Unknown Source)
at java.xml/com.sun.org.apache.xerces.internal.impl.xs.XMLSchemaValidator.handleStartElement(Unknown Source)
at java.xml/com.sun.org.apache.xerces.internal.impl.xs.XMLSchemaValidator.startElement(Unknown Source)
at java.xml/com.sun.org.apache.xerces.internal.jaxp.validation.DOMValidatorHelper.beginNode(Unknown Source)
at java.xml/com.sun.org.apache.xerces.internal.jaxp.validation.DOMValidatorHelper.validate(Unknown Source)
at java.xml/com.sun.org.apache.xerces.internal.jaxp.validation.DOMValidatorHelper.validate(Unknown Source)
at java.xml/com.sun.org.apache.xerces.internal.jaxp.validation.ValidatorImpl.validate(Unknown Source)
at java.xml/javax.xml.validation.Validator.validate(Unknown Source)
at org.apache.activemq.artemis.utils.XMLUtil.validate(XMLUtil.java:361)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3564: NO-JIRA remove ordering constraint from tag in XML
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3564:
URL: https://github.com/apache/activemq-artemis/pull/3564#discussion_r627172760
##########
File path: examples/features/clustered/clustered-jgroups/src/main/resources/activemq/server1/broker.xml
##########
@@ -42,9 +42,9 @@ under the License.
<broadcast-groups>
<broadcast-group name="my-broadcast-group">
- <broadcast-period>5000</broadcast-period>
<jgroups-file>test-jgroups-file_ping.xml</jgroups-file>
<jgroups-channel>active_broadcast_channel</jgroups-channel>
+ <broadcast-period>5000</broadcast-period>
Review comment:
Depends on the test, if its a very small unit within the module itself. But in general theres actually a tests module where tests are divided into type, which will run with different maven profiles.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3564: NO-JIRA remove ordering constraint from tag in XML
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3564:
URL: https://github.com/apache/activemq-artemis/pull/3564#discussion_r625515685
##########
File path: examples/features/clustered/clustered-jgroups/src/main/resources/activemq/server1/broker.xml
##########
@@ -42,9 +42,9 @@ under the License.
<broadcast-groups>
<broadcast-group name="my-broadcast-group">
- <broadcast-period>5000</broadcast-period>
<jgroups-file>test-jgroups-file_ping.xml</jgroups-file>
<jgroups-channel>active_broadcast_channel</jgroups-channel>
+ <broadcast-period>5000</broadcast-period>
Review comment:
This is an unnecessary change
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ehossack-aws commented on pull request #3564: NO-JIRA remove ordering constraint from tag in XML
Posted by GitBox <gi...@apache.org>.
ehossack-aws commented on pull request #3564:
URL: https://github.com/apache/activemq-artemis/pull/3564#issuecomment-848079124
Okay, thanks! For the record I would've been happy to do this, but didn't have the context so was asking for clarification.
I will close https://issues.apache.org/jira/browse/ARTEMIS-3284
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on pull request #3564: NO-JIRA remove ordering constraint from tag in XML
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on pull request #3564:
URL: https://github.com/apache/activemq-artemis/pull/3564#issuecomment-831689410
@ehossack-aws there shouldnt be an open ended jira. You should raise a jira addressing each issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on pull request #3564: NO-JIRA remove ordering constraint from tag in XML
Posted by GitBox <gi...@apache.org>.
jbertram commented on pull request #3564:
URL: https://github.com/apache/activemq-artemis/pull/3564#issuecomment-831594541
A lot of the XML is like this. I think a proper Jira would be good to track this as well as any other related future changes that are needed to make configuration more friendly. Also, an actual test-case would be good to have here instead of just the change in the example. There are a handful of XML parsing tests which could potentially change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ehossack-aws commented on a change in pull request #3564: NO-JIRA remove ordering constraint from tag in XML
Posted by GitBox <gi...@apache.org>.
ehossack-aws commented on a change in pull request #3564:
URL: https://github.com/apache/activemq-artemis/pull/3564#discussion_r625977461
##########
File path: examples/features/clustered/clustered-jgroups/src/main/resources/activemq/server1/broker.xml
##########
@@ -42,9 +42,9 @@ under the License.
<broadcast-groups>
<broadcast-group name="my-broadcast-group">
- <broadcast-period>5000</broadcast-period>
<jgroups-file>test-jgroups-file_ping.xml</jgroups-file>
<jgroups-channel>active_broadcast_channel</jgroups-channel>
+ <broadcast-period>5000</broadcast-period>
Review comment:
Will do. I was verifying the functionality works since it didn't appear there were lots of tests.
To your knowledge is there a good place for the tests to live?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on pull request #3564: NO-JIRA remove ordering constraint from tag in XML
Posted by GitBox <gi...@apache.org>.
jbertram commented on pull request #3564:
URL: https://github.com/apache/activemq-artemis/pull/3564#issuecomment-841433225
The current configuration semantics must continue to be supported at the very least for compatibility reasons so a documentation change would be the recommended approach here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ehossack-aws commented on pull request #3564: NO-JIRA remove ordering constraint from tag in XML
Posted by GitBox <gi...@apache.org>.
ehossack-aws commented on pull request #3564:
URL: https://github.com/apache/activemq-artemis/pull/3564#issuecomment-831600696
> I think a proper Jira would be good to track this as well as any other related future changes that are needed to make configuration more friendly.
It seems like there's a lot of areas of improvement in the config and a Jira ticket might just be one of those indefinitely-open issues?
Happy to file it and note things down, but I did want to raise this cr as a small improvement.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ehossack-aws commented on pull request #3564: NO-JIRA remove ordering constraint from tag in XML
Posted by GitBox <gi...@apache.org>.
ehossack-aws commented on pull request #3564:
URL: https://github.com/apache/activemq-artemis/pull/3564#issuecomment-832120434
> @ehossack-aws there shouldnt be an open ended jira. You should raise a jira addressing each issue.
Will create one!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ehossack-aws commented on pull request #3564: NO-JIRA remove ordering constraint from tag in XML
Posted by GitBox <gi...@apache.org>.
ehossack-aws commented on pull request #3564:
URL: https://github.com/apache/activemq-artemis/pull/3564#issuecomment-831599097
> Also, an actual test-case would be good to have here instead of just the change in the example. There are a handful of XML parsing tests which could potentially change.
Yeah, and it seems like we have failures in this PR regardless so we shouldn't move forward - an unbounded number of connector refs makes sense to me I think?
This could be worth just updating in the docs instead of making it order independent - what do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3564: NO-JIRA remove ordering constraint from tag in XML
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3564:
URL: https://github.com/apache/activemq-artemis/pull/3564#discussion_r625515685
##########
File path: examples/features/clustered/clustered-jgroups/src/main/resources/activemq/server1/broker.xml
##########
@@ -42,9 +42,9 @@ under the License.
<broadcast-groups>
<broadcast-group name="my-broadcast-group">
- <broadcast-period>5000</broadcast-period>
<jgroups-file>test-jgroups-file_ping.xml</jgroups-file>
<jgroups-channel>active_broadcast_channel</jgroups-channel>
+ <broadcast-period>5000</broadcast-period>
Review comment:
This is an unnecessary change, pls revert
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ehossack-aws commented on pull request #3564: NO-JIRA remove ordering constraint from tag in XML
Posted by GitBox <gi...@apache.org>.
ehossack-aws commented on pull request #3564:
URL: https://github.com/apache/activemq-artemis/pull/3564#issuecomment-841531335
@jbertram I understand. That implies that if a broadcast groups contains multiple connector-refs that's a valid configuration?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ehossack-aws commented on pull request #3564: NO-JIRA remove ordering constraint from tag in XML
Posted by GitBox <gi...@apache.org>.
ehossack-aws commented on pull request #3564:
URL: https://github.com/apache/activemq-artemis/pull/3564#issuecomment-832121090
Can either of you @jbertram or @michaelandrepearce confirm that it makes sense to allow any number of connector refs in broadcast groups? That intuitively makes sense to me, to broadcast ones connectors, but if that's the case I will adjust the PR to be about documentation rather than XSD.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ehossack-aws commented on pull request #3564: NO-JIRA remove ordering constraint from tag in XML
Posted by GitBox <gi...@apache.org>.
ehossack-aws commented on pull request #3564:
URL: https://github.com/apache/activemq-artemis/pull/3564#issuecomment-833101654
I have created https://issues.apache.org/jira/browse/ARTEMIS-3284 and will link whichever approach to the commit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram closed pull request #3564: NO-JIRA remove ordering constraint from tag in XML
Posted by GitBox <gi...@apache.org>.
jbertram closed pull request #3564:
URL: https://github.com/apache/activemq-artemis/pull/3564
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on pull request #3564: NO-JIRA remove ordering constraint from tag in XML
Posted by GitBox <gi...@apache.org>.
jbertram commented on pull request #3564:
URL: https://github.com/apache/activemq-artemis/pull/3564#issuecomment-847270706
@ehossack-aws, it's valid in the sense that it's technically _possible_ and therefore should continue to be supported. However, from what I can tell this looks like a bit of left-over configuration from when the clustering implementation worked a bit differently than it does now so in that sense I don't think such a configuration is valid. I doubt anyone is actually using multiple connectors, and we don't have any tests for such a configuration.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] gemmellr commented on pull request #3564: NO-JIRA remove ordering constraint from tag in XML
Posted by GitBox <gi...@apache.org>.
gemmellr commented on pull request #3564:
URL: https://github.com/apache/activemq-artemis/pull/3564#issuecomment-833400064
I believe he is waiting for clarity around which approach the commit should now take (i.e changing the docs, or the xsd) as mentioned here/the JIRA, before updating the commit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on pull request #3564: NO-JIRA remove ordering constraint from tag in XML
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on pull request #3564:
URL: https://github.com/apache/activemq-artemis/pull/3564#issuecomment-833322169
See hacking guide for all details of expectations of contribution, re commit message see in particular https://activemq.apache.org/components/artemis/documentation/2.0.0/hacking-guide/code.html#commitMessageDetails
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on pull request #3564: NO-JIRA remove ordering constraint from tag in XML
Posted by GitBox <gi...@apache.org>.
jbertram commented on pull request #3564:
URL: https://github.com/apache/activemq-artemis/pull/3564#issuecomment-848037133
I went ahead and just fixed the doc for this: https://github.com/apache/activemq-artemis/commit/295cf7996b8776f6fcbcf3c368aaef82ba6f802a.
If we want to change this so that order is not enforced then we'll need to deprecate the current configuration and change it in the next major version where breaking changes are allowed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org