gemmellr commented on code in PR #4938:
URL: https://github.com/apache/activemq-artemis/pull/4938#discussion_r1604702110
##########
docs/user-manual/federation-address.adoc:
##########
@@ -112,96 +111,97 @@ Sample Address Federation setup:
</federations>
----
-In the above setup downstream broker `eu-north-1` is configured to connect to
two upstream brokers `eu-east-1` and `eu-east-2`, the credentials used for both
connections to both brokers in this sample are shared, you can set user and
password at the upstream level should they be different per upstream.
+In the above setup, downstream broker `eu-north-1` is configured to connect to
two upstream brokers `eu-east-1` and `eu-west-1`. Credentials used for both
connections to brokers in this sample are shared.
+Should they be different per upstream, you can alter credentials at the
upstream level.
-Both upstreams are configured with the same address-policy
`news-address-federation`, that is selecting addresses which match any of the
include criteria, but will exclude anything that starts `queue.news.sport`.
+Both upstreams are configured with the same address-policy
`news-address-federation`, that is selecting addresses which match any of the
include criteria and exclude anything that starts `queue.news.sport`.
*It is important that federation name is globally unique.*
-Let's take a look at all the `address-policy` parameters in turn, in order of
priority.
+==== Priority ordered address-policy parameters
name::
All address-policies must have a unique name in the server.
+
include::
-The address-match pattern to include addresses.
-Multiple of these can be set.
-If none are set all addresses are matched.
+The address-match pattern to include addresses. Multiple of these can be set.
If none are set all addresses are matched.
+
exclude::
-The address-match pattern to exclude addresses.
-Multiple of these can be set.
+The address-match pattern to exclude addresses. Multiple of these can be set.
+
max-hops::
-The number of hops that a message can have made for it to be federated, see
<<topology-patterns,Topology Patterns>> above for more details.
+The maximum number of hops that message can perform across federated
addresses. See <<topology-patterns,Topology Patterns>> above for details.
auto-delete::
For address federation, the downstream dynamically creates a durable queue on
the upstream address.
-This is used to mark if the upstream queue should be deleted once downstream
disconnects, and the delay and message count params have been met.
-This is useful if you want to automate the clean up, though you may wish to
disable this if you want messages to queued for the downstream when disconnect
no matter what.
+This is used to mark if the upstream queue should be deleted once downstream
disconnects and the delay and message count params have been met. It is useful
in automation of the cleanup. You may wish to disable this, if you want
messages to be queued for the downstream when disconnect no matter what.
auto-delete-delay::
-The amount of time in milliseconds after the downstream broker has
disconnected before the upstream queue can be eligable for `auto-delete`.
+The amount of time in milliseconds after the downstream broker has
disconnected, before the upstream queue can be eligible for `auto-delete`.
auto-delete-message-count::
-The amount number messages in the upstream queue that the message count must
be equal or below before the downstream broker has disconnected before the
upstream queue can be eligable for `auto-delete`.
+Maximum number of messages allowed in the upstream queue to be eligible for
`auto-delete`, after the downstream broker has disconnected.
transformer-ref::
-The ref name for a transformer (see transformer config) that you may wish to
configure to transform the message on federation transfer.
+Reference name of a transformer (see transformer config) that you may wish to
configure to transform the message on federation transfer.
enable-divert-bindings::
-Setting to true will enable divert bindings to be listened for demand.
-If there is a divert binding with an address that matches the included
addresses for the stream, any queue bindings that match the forward address of
the divert will create demand.
-Default is `false`.
+Setting to `true` will enable divert bindings to be listened for demand.
+If there is a divert binding with an address that matches the included
addresses for the stream, any queue bindings that match the forward address of
the divert will create demand. Default is `false`.
NOTE: `address-policy` and `queue-policy` elements can be defined in the same
federation and be linked to the same upstream.
-Now look at all the `transformer` parameters in turn, in order of priority:
+==== Priority ordered transformer parameters
name::
-This must be a unique name in the server, and is used to ref the transformer
in `address-policy` and `queue-policy`
+Unique name in the server used to reference the transformer in
`address-policy` and `queue-policy`.
transformer-class-name::
-An optional transformer-class-name can be specified.
+An optional `transformer-class-name` can be specified.
Review Comment:
I dont really see a point in immediately highlighting the name of the
attribute, which is already much more significantly bolded / highlighted in the
overall attribute list anyway. If changing it, maybe just remove the hyphens
instead? Or combine the 'optional' bit into the similar sentence that follows.
##########
docs/user-manual/federation-address.adoc:
##########
@@ -112,96 +111,97 @@ Sample Address Federation setup:
</federations>
----
-In the above setup downstream broker `eu-north-1` is configured to connect to
two upstream brokers `eu-east-1` and `eu-east-2`, the credentials used for both
connections to both brokers in this sample are shared, you can set user and
password at the upstream level should they be different per upstream.
+In the above setup, downstream broker `eu-north-1` is configured to connect to
two upstream brokers `eu-east-1` and `eu-west-1`. Credentials used for both
connections to brokers in this sample are shared.
+Should they be different per upstream, you can alter credentials at the
upstream level.
-Both upstreams are configured with the same address-policy
`news-address-federation`, that is selecting addresses which match any of the
include criteria, but will exclude anything that starts `queue.news.sport`.
+Both upstreams are configured with the same address-policy
`news-address-federation`, that is selecting addresses which match any of the
include criteria and exclude anything that starts `queue.news.sport`.
*It is important that federation name is globally unique.*
-Let's take a look at all the `address-policy` parameters in turn, in order of
priority.
+==== Priority ordered address-policy parameters
Review Comment:
I don't think this is a good heading / change (and similarly for the others
like it). Add a heading, by all means...but the fact the entries are discussed
in some 'order of usefulness' is not really something that needs to be in that
heading, and largely just distracts from the actual useful bit of the heading
(especially given the other headings starting the same way).
##########
docs/user-manual/federation-address.adoc:
##########
@@ -83,19 +82,19 @@ Sample Address Federation setup:
<upstream name="eu-east-1">
<static-connectors>
<connector-ref>eu-east-connector1</connector-ref>
- <connector-ref>eu-east-connector1</connector-ref>
+ <connector-ref>eu-east-connector2</connector-ref>
Review Comment:
You have changed connector refs in almost every config example in the doc,
fixing some apparent bugs within themselves...however those config examples all
seem to remain inconsistent with each other in terms of refs. Varying connector
name formats like "connector1", "eu-east-connector2", and "eu-east-1-connector"
all remain. Should those that seem related to an overall geographic example
perhaps actually be aligned to use the same naming format?
##########
docs/user-manual/federation-address.adoc:
##########
@@ -112,96 +111,97 @@ Sample Address Federation setup:
</federations>
----
-In the above setup downstream broker `eu-north-1` is configured to connect to
two upstream brokers `eu-east-1` and `eu-east-2`, the credentials used for both
connections to both brokers in this sample are shared, you can set user and
password at the upstream level should they be different per upstream.
+In the above setup, downstream broker `eu-north-1` is configured to connect to
two upstream brokers `eu-east-1` and `eu-west-1`. Credentials used for both
connections to brokers in this sample are shared.
+Should they be different per upstream, you can alter credentials at the
upstream level.
-Both upstreams are configured with the same address-policy
`news-address-federation`, that is selecting addresses which match any of the
include criteria, but will exclude anything that starts `queue.news.sport`.
+Both upstreams are configured with the same address-policy
`news-address-federation`, that is selecting addresses which match any of the
include criteria and exclude anything that starts `queue.news.sport`.
*It is important that federation name is globally unique.*
-Let's take a look at all the `address-policy` parameters in turn, in order of
priority.
+==== Priority ordered address-policy parameters
name::
All address-policies must have a unique name in the server.
+
include::
-The address-match pattern to include addresses.
-Multiple of these can be set.
-If none are set all addresses are matched.
+The address-match pattern to include addresses. Multiple of these can be set.
If none are set all addresses are matched.
Review Comment:
Pretty sure these were on new lines deliberately, as part of an overall
'line per sentence' intent. If so I dont think we should change it here (and
various other places), and thus also ehlp minimise the diff for this change
overall.
@jbertram would know better as he did the conversion.
EDIT: Yes, they were, original PR:
https://github.com/apache/activemq-artemis/pull/4573
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
