Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARTEMIS-4213 doc update added some Broker Properties reference and added s… #4410

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

andytaylor
Copy link
Contributor

…ome docs to the schema

@brusdev
Copy link
Member

brusdev commented Mar 22, 2023

@andytaylor could you create a JIRA to track this change?

@andytaylor andytaylor force-pushed the broker-properties-docs branch from cba4033 to c7d6050 Compare March 22, 2023 10:19
@andytaylor andytaylor changed the title NO-JIRA doc update added some Broker Properties reference and added s… ARTEMIS-4213 doc update added some Broker Properties reference and added s… Mar 22, 2023
@andytaylor andytaylor force-pushed the broker-properties-docs branch from c7d6050 to 2dd69cb Compare March 22, 2023 10:35
@@ -481,3 +484,285 @@ Name | Description | Default
[password](amqp-broker-connections.md#amqp-server-connections) | Broker authentication password (optional) | n/a
[reconnect-attempts](amqp-broker-connections.md#amqp-server-connections) | How many attempts should be made to reconnect after failure. | -1 (infinite)
[auto-start](amqp-broker-connections.md#amqp-server-connections) | Broker connection starts automatically with broker | true


## Broker Properties Reference
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This perhaps do with a small intro like the earlier sections, maybe a reference to the doc on how to actually use these properties and how their naming convention relates to the XML config and code.

Would also be worth a warning that the property values set will override existing XML config, which can in turn already have overridden any 'real code default' (which the XML does so out of the box for some things), meaning some of the below listed Default values may not actually match the no-property-defined 'effective-default' at all, since they are defined differently in the xml config they use/get (either by them previously...or right out of the box in the 'default xml config' in some cases).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As there is no specific pattern to these (as far as I recall from conversations with Gary), coming up with the explicit naming convention could prove difficult. It's one of the things I've tried to push for ...

Disclaimer about precedence / priority for application of configuration would be definitely beneficial.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation I suggested referencing outlines the naming to an extent already, https://github.com/apache/activemq-artemis/blob/2.28.0/docs/user-manual/en/configuration-index.md#broker-properties

I think even more so now that the behaviour arounds defaults need to be clearer. It came up in another context since that earlier comment, in #4297 where I eventually realised (and argued, it isnt clear to a user of it) that the 'properties image' will actually behave quite differently out-of-the-box than the standalone broker and artemis-docker based images that use the 'standard supplied XML' which already overrides various code defaults. This PRs existing docs on 'Default' would actually currently best suit/describe what happens in a 'properties-only image' case, while potentially misleading in the 'standard supplied XML image' currently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants