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 2022/06/23 21:52:36 UTC
[GitHub] [activemq-artemis] ryan-highley opened a new pull request, #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
ryan-highley opened a new pull request, #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122
Moves embedded XSD complexType definitions from XSD element definitions
to top-level types available for XML schema validation in IDEs.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r910992354
##########
artemis-server/src/main/resources/schema/artemis-configuration.xsd:
##########
@@ -16,18 +16,23 @@
limitations under the License.
-->
<xsd:schema xmlns:xsd="http://www.w3.org/2001/XMLSchema"
+ xmlns:xi="http://www.w3.org/2001/XInclude"
xmlns="urn:activemq:core"
targetNamespace="urn:activemq:core"
attributeFormDefault="unqualified"
elementFormDefault="qualified"
version="1.0">
<xsd:import namespace="http://www.w3.org/XML/1998/namespace" schemaLocation="xml.xsd"/>
+ <xsd:import namespace="http://www.w3.org/2001/XInclude" schemaLocation="XInclude.xsd"/>
<xsd:element name="core" type="configurationType"/>
<xsd:complexType name="configurationType">
<xsd:all>
+
+ <xsd:element ref="xi:include" maxOccurs="1" minOccurs="0"/>
Review Comment:
This is not needed Includes already works
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
jbertram commented on PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#issuecomment-1380799784
@michaelandrepearce, I can't even get `xmllint` to validate the normal `broker.xml` on a non-air-gapped system. Is there some trick to this? Here's what I did:
1. Created a fresh instance of Artemis 2.27.1 called `test`.
2. Ran this command from `ARTEMIS_HOME`
```
$ xmllint --valid --noout --schema schema/artemis-server.xsd path/to/test/etc/broker.xml
path/to/test/etc/broker.xml:24: validity error : Validation failed: no DTD found !
xsi:schemaLocation="urn:activemq /schema/artemis-configuration.xsd">
^
path/to/test/etc/broker.xml validates
```
Is this a success or a failure?
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r911235227
##########
artemis-server/src/test/resources/ConfigurationTest-xinclude-config-address-settings.xml:
##########
@@ -14,7 +14,10 @@
See the License for the specific language governing permissions and
limitations under the License.
-->
-<address-settings xmlns="urn:activemq:core">
+<address-settings xmlns="urn:activemq:core"
Review Comment:
should not need to have any changes to existing test cases, any such changes suggest break changes
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ryan-highley commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
ryan-highley commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r911303830
##########
artemis-server/src/test/resources/ConfigurationTest-xinclude-config-address-settings.xml:
##########
@@ -14,7 +14,10 @@
See the License for the specific language governing permissions and
limitations under the License.
-->
-<address-settings xmlns="urn:activemq:core">
+<address-settings xmlns="urn:activemq:core"
Review Comment:
As I stated, these changes are not needed for the tests to pass. These changes simply add other XML include paths to the existing test suite.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ryan-highley commented on pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
ryan-highley commented on PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#issuecomment-1178414299
@michaelandrepearce Any update or feedback from your further testing?
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#issuecomment-1380786448
@jbertram so air gapped, yes. Two things the team here found
1) xmllint didn't like it
2) we ignored our system process and bypassed xmllint pre check, and forced it in still just to see, and then upgraded broker from pre-existing 2.16 broker didn't go well. Its been 6 months now so exact error i cannot remember.
but it needs some good air gap testing, and working out why these things.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r911224533
##########
artemis-server/src/main/resources/schema/artemis-configuration.xsd:
##########
@@ -16,18 +16,23 @@
limitations under the License.
Review Comment:
As i said, if theres a specific, area that needs to change great, im happy to help focus to address the point area, but as it stands and as a project maintainer i don't feel that comfortable with this change the risk is very large and i don't feel is needed personally. If other maintainers (members) wish to jump in here, and feel very confident about the change not breaking anyone's setups then great.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r910976875
##########
artemis-server/src/test/resources/ConfigurationTest-xinclude-config-acceptors.xml:
##########
@@ -14,7 +14,10 @@
See the License for the specific language governing permissions and
limitations under the License.
-->
-<acceptors xmlns="urn:activemq:core">
+<acceptors xmlns="urn:activemq:core"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="urn:activemq:core ../../../../artemis-server/src/main/resources/schema/artemis-configuration.xsd"
Review Comment:
-1 on this should not be needed to be so explicit. this is very fragile
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ryan-highley commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
ryan-highley commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r911114380
##########
artemis-server/src/test/resources/ConfigurationTest-xinclude-config-broker-connections.xml:
##########
@@ -0,0 +1,31 @@
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+<broker-connections xmlns="urn:activemq:core">
Review Comment:
The required {{<xsd:attributeGroup ref="xml:specialAttrs"/>}} addition was added to the {{brokerConnectType}} definition.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
jbertram commented on PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#issuecomment-1355444322
@ryan-highley, the test failure was unrelated and I've pushed a fix for it. Please `git commit --amend` and push your branch again to trigger a rebuild.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ryan-highley commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
ryan-highley commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r911119201
##########
artemis-server/src/main/resources/schema/XInclude.xsd:
##########
@@ -0,0 +1,49 @@
+<xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema"
Review Comment:
Referencing {{xi:include}} in the XSD follows the same pattern established for the {{xml}} namespace for the {{xml:specialAttrs}} attribute group. See my comment on the {{xi:include}} element reference.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelpearce-gain commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r911148846
##########
artemis-server/src/main/resources/schema/XInclude.xsd:
##########
@@ -0,0 +1,49 @@
+<xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema"
Review Comment:
But as I mentioned its already theres docs how to do this. Many installs already successfully modulise their xml.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#issuecomment-1184968423
Only thought someone had in our team was if the xinclude.xsd that is added for offline need to reference xml.xsd
<xsd:import namespace="http://www.w3.org/XML/1998/namespace" schemaLocation="xml.xsd"/>
But that then seems not to be liked by lint tools
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r911224533
##########
artemis-server/src/main/resources/schema/artemis-configuration.xsd:
##########
@@ -16,18 +16,23 @@
limitations under the License.
Review Comment:
As i said, if theres a specific, area that needs to change (e.g. a particular section isn't importable but is sensible to extract) great, im happy to help focus to address the point area, but as it stands and as a project maintainer (pmc) i don't feel that comfortable with this change the risk is very large and i don't feel is needed personally. If other maintainers (pmc or committers) wish to jump in here, and feel very confident about the change not breaking anyone's setups then great.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ryan-highley commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
ryan-highley commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r911106800
##########
artemis-server/src/test/resources/ConfigurationTest-xinclude-config-acceptors.xml:
##########
@@ -14,7 +14,10 @@
See the License for the specific language governing permissions and
limitations under the License.
-->
-<acceptors xmlns="urn:activemq:core">
+<acceptors xmlns="urn:activemq:core"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="urn:activemq:core ../../../../artemis-server/src/main/resources/schema/artemis-configuration.xsd"
Review Comment:
Correct, none of these test files need to be so explicit. The {{broker-connections}} file element covers the original include namespace attribute test case matching the documentation.
I added the common namespace attribute combinations to the existing xinclude element test files to ensure adequate testing of includes with the updated XSD. This file and the {{address-settings}} include explicitly define the schema definition location, just as {{ConfigurationTest-xinclude-config.xml}} does, as well as an explicit type reference attribute. The {{addresses}} file skips the explicit type reference. The {{broker-connections}} file include sticks with the original, bare {{xmlns}} URN.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ryan-highley commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
ryan-highley commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r911106800
##########
artemis-server/src/test/resources/ConfigurationTest-xinclude-config-acceptors.xml:
##########
@@ -14,7 +14,10 @@
See the License for the specific language governing permissions and
limitations under the License.
-->
-<acceptors xmlns="urn:activemq:core">
+<acceptors xmlns="urn:activemq:core"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="urn:activemq:core ../../../../artemis-server/src/main/resources/schema/artemis-configuration.xsd"
Review Comment:
Correct, none of these test files need to be so explicit. The {{broker-connections}} file element covers the original include namespace attribute test case matching the documentation.
I added the common namespace attribute combinations to the existing xinclude element test files to ensure adequate testing of includes with the updated XSD. This file and the {{address-settings}} include file explicitly define the schema definition location, just as {{ConfigurationTest-xinclude-config.xml}} does, as well as an explicit type reference attribute. The {{addresses}} file skips the explicit type reference. The {{broker-connections}} file include sticks with the original, bare {{xmlns}} URN.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ryan-highley commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
ryan-highley commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r911073599
##########
artemis-server/src/main/resources/schema/artemis-configuration.xsd:
##########
@@ -16,18 +16,23 @@
limitations under the License.
Review Comment:
The intent of the changes is to remove all the embedded {{element}} and {{complexType}} definitions from the {{configurationType}} definition.
By moving these schema definitions, XML tooling becomes valid for both included XML elements and the overall {{broker.xml}} file. As with the {{diverts}} example in the JIRA (not mine--just trying to address it), an IDE has no idea what a {{diverts}} is outside of a {{core}} element, meaning syntax help is not available. Any other valid imported element file, e.g. included files containing top-level {{addresses}}, {{connectors}}, {{acceptors}}, etc. elements, would also have the same syntax tooling support.
As you mentioned, XML includes work just fine currently, but the schema definition as it currently stands does not support included element files as well as it can and should.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ryan-highley commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
ryan-highley commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r911237373
##########
artemis-server/src/test/resources/ConfigurationTest-xinclude-config-address-settings.xml:
##########
@@ -14,7 +14,10 @@
See the License for the specific language governing permissions and
limitations under the License.
-->
-<address-settings xmlns="urn:activemq:core">
+<address-settings xmlns="urn:activemq:core"
Review Comment:
As shown in the broker-connections and security-settings test files, the additional namespace attributes are not necessary and do not represent a breaking 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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#issuecomment-1398636651
@jbertram if latest is working for you, then i can have to assume it would for us, original version didn't. Unfortunately i spent all my internal credits on this PR early on testing it. If you're sure that its good then i remove my -1, to a 0, and let you merge if you're happy.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#issuecomment-1194108106
> @michaelandrepearce Finally able to circle back to this. (Stupid day job....)
>
> Could you attach your config files so I can attempt to recreate the issue?
I cannot, it's an air gapped systems for a reason. I can only suggest you try make an air gapped setup and see if you can recreate the issues.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelpearce-gain commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r911147124
##########
artemis-server/src/main/resources/schema/artemis-configuration.xsd:
##########
@@ -16,18 +16,23 @@
limitations under the License.
-->
<xsd:schema xmlns:xsd="http://www.w3.org/2001/XMLSchema"
+ xmlns:xi="http://www.w3.org/2001/XInclude"
xmlns="urn:activemq:core"
targetNamespace="urn:activemq:core"
attributeFormDefault="unqualified"
elementFormDefault="qualified"
version="1.0">
<xsd:import namespace="http://www.w3.org/XML/1998/namespace" schemaLocation="xml.xsd"/>
+ <xsd:import namespace="http://www.w3.org/2001/XInclude" schemaLocation="XInclude.xsd"/>
<xsd:element name="core" type="configurationType"/>
<xsd:complexType name="configurationType">
<xsd:all>
+
+ <xsd:element ref="xi:include" maxOccurs="1" minOccurs="0"/>
Review Comment:
Against the jira thats linked here the need is to have divert in an external config file imported.... this is possible today without these mass changes
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelpearce-gain commented on pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#issuecomment-1172844670
Im just testing this in some real env sandboxes. Something is breaking when doing a cluster upgrade with this change, trying to figure what its not liking to feed back.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r910977942
##########
artemis-server/src/test/resources/ConfigurationTest-xinclude-config-broker-connections.xml:
##########
@@ -0,0 +1,31 @@
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+<broker-connections xmlns="urn:activemq:core">
Review Comment:
If we want broker connection to support include, can we please just follow the existing solution for how we support i for other key config parts
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r910996139
##########
artemis-server/src/main/resources/schema/artemis-configuration.xsd:
##########
@@ -16,18 +16,23 @@
limitations under the License.
Review Comment:
Not sure whats going on with soo many changes here, the JIRA mentions diverts, this is already supported to be able to include in a deployed configuration setup (follow docs) i know many installs in the real world already doing this today.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ryan-highley commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
ryan-highley commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r911477101
##########
artemis-server/src/test/resources/ConfigurationTest-xinclude-config-address-settings.xml:
##########
@@ -14,7 +14,10 @@
See the License for the specific language governing permissions and
limitations under the License.
-->
-<address-settings xmlns="urn:activemq:core">
+<address-settings xmlns="urn:activemq:core"
Review Comment:
Reverted all changes to the prior ConfigurationFile-xinclude-....xml unit test resource files. Added a separate unit test class and many modular include files for all top-level, non-trivial element types while still passing the existing FileConfigurationTest and ConfigurationImplTest scenarios.
The ConfigurationImplTest#testSerialize() method makes a questionable assumption regarding broker plugins in test configurations but I worked around it to preserve the existing unit test while also including a modular broker plugins file.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#issuecomment-1171180182
I see in your JIRA you say you want to have diverts separated, this already works, in fact i know a few orgs where this is done successfully. If you are running lint tools, for validation as per the aforementioned docs,
"
Note: if you use xmllint to validate the XML against the schema you should enable xinclude flag when running.
--xinclude
"
As such if you're using an IDE you just need to add this to your lint'ing.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r911239569
##########
artemis-server/src/test/resources/ConfigurationTest-xinclude-config-address-settings.xml:
##########
@@ -14,7 +14,10 @@
See the License for the specific language governing permissions and
limitations under the License.
-->
-<address-settings xmlns="urn:activemq:core">
+<address-settings xmlns="urn:activemq:core"
Review Comment:
existing test cases and configs should remain untouched.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ryan-highley commented on pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
ryan-highley commented on PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#issuecomment-1194102239
@michaelandrepearce Finally able to circle back to this. (Stupid day job....)
Could you attach your config files so I can attempt to recreate the 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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r911387589
##########
artemis-server/src/main/resources/schema/artemis-configuration.xsd:
##########
@@ -16,18 +16,23 @@
limitations under the License.
-->
<xsd:schema xmlns:xsd="http://www.w3.org/2001/XMLSchema"
+ xmlns:xi="http://www.w3.org/2001/XInclude"
xmlns="urn:activemq:core"
targetNamespace="urn:activemq:core"
attributeFormDefault="unqualified"
elementFormDefault="qualified"
version="1.0">
<xsd:import namespace="http://www.w3.org/XML/1998/namespace" schemaLocation="xml.xsd"/>
+ <xsd:import namespace="http://www.w3.org/2001/XInclude" schemaLocation="XInclude.xsd"/>
Review Comment:
should we lower case this case this for consistency with the existing xml.xsd e.g. xinclude.xsd, also avoids future fun with file case sensitivity we've seen on some systems
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r911160813
##########
artemis-server/src/main/resources/schema/artemis-configuration.xsd:
##########
@@ -16,18 +16,23 @@
limitations under the License.
Review Comment:
But point is, modular is already supported and many key parts that most common users want out are and are used in many production systems successfully... if theres a section that is sensible to be modular and isn't then we should focus on that, but this very large change (any large change is high risk, as you never know how it could impact others) so i would rather we focus on just what isn't possible to be modularised today, if its sensible to do so.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ryan-highley commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
ryan-highley commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r911219489
##########
artemis-server/src/main/resources/schema/artemis-configuration.xsd:
##########
@@ -16,18 +16,23 @@
limitations under the License.
Review Comment:
And I understand your point. I work building large-scale production Artemis systems for clients every day--that's my job. I use modular files for common sections of broker.xml configurations extensively to build everything from cluster prototypes to multi-DC, world-wide messaging infrastructure.
My point is simple. The XSD does not support modularity as well as it should due to the "anonymous" element and complexType definitions buried in the configurationType definition. This is bad XSD form as the embedded defined types and elements cannot be used directly--they're only useful in the context of the outermost defining type, the configurationType in this case.
Moving these embedded elements and their associated complex types to top-level definitions enables XML tooling to recognize and support the schema definition in both modular files as they're being developed as well as complete broker.xml files. While the elements may not make sense outside a broker configuration, supporting modular files necessarily implies the elements therein are portable to those files. The same tooling support should be available in the modular files as is available when developing a monolithic broker.xml file.
I agree the scale of this change involves significant risk. This is why I included additional updates to the existing xinclude unit tests to address that risk. If you would like to suggest additional test scenarios, I'm happy to include those as well.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
jbertram commented on PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#issuecomment-1380782116
@michaelandrepearce, so in order to reproduce the issue I need to create an instance of the broker on an air-gapped system using the schema updates in this PR and then run `xmllint` on `broker.xml`?
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#issuecomment-1381456192
--valid is for DTD validation as such it expects DTD's to be given. were doing XSD which you're correctly passing schema, remove --valid
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
jbertram commented on PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#issuecomment-1396363992
I performed an upgrade from 2.23.0 to the release I built from this PR and everything worked as expected. The `broker.xml` also validates fine with `xmllint`. @michaelandrepearce, what are your thoughts here? Any chance you could perform your own testing to confirm everything works as expected and if not provided specific details of what failed?
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#issuecomment-1171173775
Please see https://activemq.apache.org/components/artemis/documentation/latest/configuration-index.html we already support Modularising the broker xml with import
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ryan-highley commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
ryan-highley commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r911106800
##########
artemis-server/src/test/resources/ConfigurationTest-xinclude-config-acceptors.xml:
##########
@@ -14,7 +14,10 @@
See the License for the specific language governing permissions and
limitations under the License.
-->
-<acceptors xmlns="urn:activemq:core">
+<acceptors xmlns="urn:activemq:core"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="urn:activemq:core ../../../../artemis-server/src/main/resources/schema/artemis-configuration.xsd"
Review Comment:
Correct, none of these test files need to be so explicit. The {{broker-connections}} file element covers the original include namespace attribute test case matching the documentation.
I added the common namespace attribute combinations to the existing xinclude element test files to ensure adequate testing of includes with the updated XSD. This file and the {{address-settings}} include file explicitly define the schema definition location, just as {{ConfigurationTest-xinclude-config.xml}} does, as well as an explicit type reference attribute. The {{addresses}} file skips the explicit type reference attribute. The {{broker-connections}} file include sticks with the original, bare {{xmlns}} URN.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r911160813
##########
artemis-server/src/main/resources/schema/artemis-configuration.xsd:
##########
@@ -16,18 +16,23 @@
limitations under the License.
Review Comment:
But point is, modular is already supported and many key parts that most common users want out are... if theres a section that is sensible to be modular and isn't then we should focus on that, but this very large change (any large change is high risk, as you never know how it could impact others) so i would rather we focus on just what isn't possible to be modularised today, if its sensible to do so.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r911224533
##########
artemis-server/src/main/resources/schema/artemis-configuration.xsd:
##########
@@ -16,18 +16,23 @@
limitations under the License.
Review Comment:
As i said, if theres a specific, area that needs to change great, im happy to help focus to address the point area, but as it stands and as a project maintainer (pmc) i don't feel that comfortable with this change the risk is very large and i don't feel is needed personally. If other maintainers (pmc or committers) wish to jump in here, and feel very confident about the change not breaking anyone's setups then great.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ryan-highley commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
ryan-highley commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r911250825
##########
artemis-server/src/main/resources/schema/artemis-configuration.xsd:
##########
@@ -16,18 +16,23 @@
limitations under the License.
Review Comment:
If you would like me to replicate the original test files with an alternate modular include unit test scenario, I will do so. All the original test files work unaltered with the updated schema definition, as evidenced by the broker-connection and security-settings files. The existing config changes were to avoid creating a separate unit test class, which I will now create and submit.
The attached screenshot shows one example of the IDE syntax support these schema definition changes enable when developing individual modular files, not just broker.xml files.
I would welcome other input here as well.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r910976378
##########
artemis-server/src/main/resources/schema/XInclude.xsd:
##########
@@ -0,0 +1,49 @@
+<xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema"
Review Comment:
There is no need for this, we already support xincludes.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
jbertram commented on PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#issuecomment-1355518409
@michaelandrepearce, are you still -1 on this change? It looks good to me.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r910977942
##########
artemis-server/src/test/resources/ConfigurationTest-xinclude-config-broker-connections.xml:
##########
@@ -0,0 +1,31 @@
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+<broker-connections xmlns="urn:activemq:core">
Review Comment:
If we want broker connection to support include, can we please just follow the existing solution for how we support it for other key config parts
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ryan-highley commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
ryan-highley commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r911084581
##########
artemis-server/src/main/resources/schema/artemis-configuration.xsd:
##########
@@ -16,18 +16,23 @@
limitations under the License.
-->
<xsd:schema xmlns:xsd="http://www.w3.org/2001/XMLSchema"
+ xmlns:xi="http://www.w3.org/2001/XInclude"
xmlns="urn:activemq:core"
targetNamespace="urn:activemq:core"
attributeFormDefault="unqualified"
elementFormDefault="qualified"
version="1.0">
<xsd:import namespace="http://www.w3.org/XML/1998/namespace" schemaLocation="xml.xsd"/>
+ <xsd:import namespace="http://www.w3.org/2001/XInclude" schemaLocation="XInclude.xsd"/>
<xsd:element name="core" type="configurationType"/>
<xsd:complexType name="configurationType">
<xsd:all>
+
+ <xsd:element ref="xi:include" maxOccurs="1" minOccurs="0"/>
Review Comment:
{{<xi:include>}} is not currently a valid element in {{configurationType}}, leading syntax validation tooling to balk. Including this element ref is the reason for adding the XInclude.xsd file.
XSD 1.0 only allows {{all}} groups to have {{minOccurs == 0}} and {{maxOccurs == 1}}, while valid configurations of course could have multiple included element files. XSD 1.1 drops this restriction but including an XSD 1.1 validator turned out to be exceedingly painful.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r911387589
##########
artemis-server/src/main/resources/schema/artemis-configuration.xsd:
##########
@@ -16,18 +16,23 @@
limitations under the License.
-->
<xsd:schema xmlns:xsd="http://www.w3.org/2001/XMLSchema"
+ xmlns:xi="http://www.w3.org/2001/XInclude"
xmlns="urn:activemq:core"
targetNamespace="urn:activemq:core"
attributeFormDefault="unqualified"
elementFormDefault="qualified"
version="1.0">
<xsd:import namespace="http://www.w3.org/XML/1998/namespace" schemaLocation="xml.xsd"/>
+ <xsd:import namespace="http://www.w3.org/2001/XInclude" schemaLocation="XInclude.xsd"/>
Review Comment:
should we lower case this case this for consistency with the existing xml.xsd (and other xsd files) e.g. xinclude.xsd, also avoids future fun with file case sensitivity we've seen on some systems
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ryan-highley commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
ryan-highley commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r911073599
##########
artemis-server/src/main/resources/schema/artemis-configuration.xsd:
##########
@@ -16,18 +16,23 @@
limitations under the License.
Review Comment:
The intent of the changes is to remove all the embedded {{element}} and {{complexType}} definitions from the {{core}} definition.
By moving these schema definitions, XML tooling becomes valid for both included XML elements and the overall {{broker.xml}} file. As with the {{diverts}} example in the JIRA (not mine--just trying to address it), an IDE has no idea what a {{diverts}} is outside of a {{core}} element, meaning syntax help is not available. Any other valid imported element file, e.g. included files containing top-level {{addresses}}, {{connectors}}, {{acceptors}}, etc. elements, would also have the same syntax tooling support.
As you mentioned, XML includes work just fine currently, but the schema definition as it currently stands does not support included element files as well as it can and should.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r911224533
##########
artemis-server/src/main/resources/schema/artemis-configuration.xsd:
##########
@@ -16,18 +16,23 @@
limitations under the License.
Review Comment:
As i said, if theres a specific, area that needs to change (e.g. a particular section isn't importable but is sensible to extract) great, im happy to help focus to address the point area, but as it stands and as a project maintainer (pmc) i don't feel that comfortable with this change the risk is very large and i don't feel is needed personally. If other maintainers (pmc or committers) wish to jump in here, and feel very confident about the change not breaking anyone's setups then great. Though the fact existing configs for tests is needed to be changed to support the change, suggests this change is a break change. The change should not require any existing tests or configs to have 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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelpearce-gain commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r911145646
##########
artemis-server/src/main/resources/schema/artemis-configuration.xsd:
##########
@@ -16,18 +16,23 @@
limitations under the License.
Review Comment:
Im struggling here to see the issue, the elements are importable. The exact issue or want in the jira ticket having Diverts as a separate xml file is possible today.....
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ryan-highley commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
ryan-highley commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r911157566
##########
artemis-server/src/main/resources/schema/artemis-configuration.xsd:
##########
@@ -16,18 +16,23 @@
limitations under the License.
Review Comment:
Diverts is given as an example in the JIRA, not as the totality of required changes.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#issuecomment-1380754439
@jbertram has anything been done to fix the airgap 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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#issuecomment-1380771228
@jbertram so in a true fully air gapped environment linux tools like xmllint don't seem to like this. Which is used quite a bit in places to validate xml changes before applying to broker to ensure change doesn't foobar running system
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r911239281
##########
artemis-server/src/test/resources/ConfigurationTest-xinclude-config-address-settings.xml:
##########
@@ -14,7 +14,10 @@
See the License for the specific language governing permissions and
limitations under the License.
-->
-<address-settings xmlns="urn:activemq:core">
+<address-settings xmlns="urn:activemq:core"
Review Comment:
then there should not have been a need for any change to these files.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ryan-highley commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
ryan-highley commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r911323250
##########
artemis-server/src/test/resources/ConfigurationTest-xinclude-config-address-settings.xml:
##########
@@ -14,7 +14,10 @@
See the License for the specific language governing permissions and
limitations under the License.
-->
-<address-settings xmlns="urn:activemq:core">
+<address-settings xmlns="urn:activemq:core"
Review Comment:
I'm currently working on a commit that does just that.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelpearce-gain commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r911321833
##########
artemis-server/src/test/resources/ConfigurationTest-xinclude-config-address-settings.xml:
##########
@@ -14,7 +14,10 @@
See the License for the specific language governing permissions and
limitations under the License.
-->
-<address-settings xmlns="urn:activemq:core">
+<address-settings xmlns="urn:activemq:core"
Review Comment:
So don't change them.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ryan-highley commented on a diff in pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
ryan-highley commented on code in PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#discussion_r911420658
##########
artemis-server/src/main/resources/schema/artemis-configuration.xsd:
##########
@@ -16,18 +16,23 @@
limitations under the License.
-->
<xsd:schema xmlns:xsd="http://www.w3.org/2001/XMLSchema"
+ xmlns:xi="http://www.w3.org/2001/XInclude"
xmlns="urn:activemq:core"
targetNamespace="urn:activemq:core"
attributeFormDefault="unqualified"
elementFormDefault="qualified"
version="1.0">
<xsd:import namespace="http://www.w3.org/XML/1998/namespace" schemaLocation="xml.xsd"/>
+ <xsd:import namespace="http://www.w3.org/2001/XInclude" schemaLocation="XInclude.xsd"/>
Review Comment:
Good idea. I just used the file directly from w3c.org as downloaded. I've updated the included file to be lowercase.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ryan-highley commented on pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
ryan-highley commented on PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#issuecomment-1355455622
Thanks @jbertram
Noticed several curious build failures in a row. I've incorporated the updated test logic and another build is underway.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic merged pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic merged PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
jbertram commented on PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#issuecomment-1380764154
@michaelandrepearce, I haven't done any direct testing, just code review. Can you elaborate on what you mean by "tested fully"? You mentioned doing an upgrade previously. Is that what you mean? Or is it sufficient to just create an instance and start it up?
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ryan-highley commented on pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
ryan-highley commented on PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#issuecomment-1385612108
I removed the XInclude.xsd file and references in the updated schema definitions some time ago. Hopefully that appeases xmllint.
Was never able to reproduce any issues with air-gapped hosts, but without a better idea of what steps caused the issues in the first place, I won't know how to address them.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on pull request #4122: ARTEMIS-3139 Anonymous types in Artemis config XSD
Posted by GitBox <gi...@apache.org>.
jbertram commented on PR #4122:
URL: https://github.com/apache/activemq-artemis/pull/4122#issuecomment-1387349068
I created a release from this PR and copied it to an air-gapped system. I created an instance of the broker and then modified that instance's `broker.xml` to use an `xinclude`. Then I ran this from Artemis' home directory:
```
$ xmllint --noout --xinclude --schema schema/artemis-server.xsd /path/to/etc/broker.xml
/path/to/etc/broker.xml validates
```
As you can see, the validation succeeded. I will try performing an upgrade next.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org