Skip to content

Add property file-based initialization #329

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

Merged
merged 6 commits into from
Nov 10, 2017

Conversation

acogoluegnes
Copy link
Contributor

Fixes #324

* @see ConnectionFactoryConfigurer
*/
public void load(String propertyFileLocation) throws IOException {
ConnectionFactoryConfigurer.load(this, propertyFileLocation);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think Configurator is a more common word in English.

/**
* Load settings from a property file.
* @param propertyFileLocation location of the property file to use
* @param prefix prefix used in the property file keys
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not really clear from this line how the prefix is used.

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps a "see ConnectionFactoryConfigurer" link would help the most here.

public static final String PORT = "port";
public static final String REQUESTED_CHANNEL_MAX = "requested.channel.max";
public static final String REQUESTED_FRAME_MAX = "requested.frame.max";
public static final String REQUESTED_HEARTBEAT = "requested.heartbeat";
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd use "heartbeat.timeout" here. Same for frame.max and channel.max.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Again, I tried to map with the existing properties in ConnectionFactory. Consistency over better naming? :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

It's just weird that we use dots so aggressively. In case of requested.heartbeat we propagate an unfortunately named property and make it even more confusing by using dot separators.

Can we use connection.channel_max, connection.frame_max, connection.heartbeat.timeout for those 3?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure.


public static final String USERNAME = "username";
public static final String PASSWORD = "password";
public static final String VIRTUAL_HOST = "virtual.host";
Copy link
Contributor

Choose a reason for hiding this comment

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

vhost?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Property name is virtualHost in ConnectionFactory, so I'm trying to be consistent. WDYT?

public static final String SHUTDOWN_TIMEOUT = "shutdown.timeout";
public static final String USE_DEFAULT_CLIENT_PROPERTIES = "use.default.client.properties";
public static final String CLIENT_PROPERTIES_PREFIX = "client.properties.";
public static final String AUTOMATIC_RECOVERY_ENABLED = "automatic.recovery.enabled";
Copy link
Contributor

Choose a reason for hiding this comment

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

This can probably be connection.recovery.enabled. We already have topology.recovery.enabled.

public static final String CLIENT_PROPERTIES_PREFIX = "client.properties.";
public static final String AUTOMATIC_RECOVERY_ENABLED = "automatic.recovery.enabled";
public static final String TOPOLOGY_RECOVERY_ENABLED = "topology.recovery.enabled";
public static final String NETWORK_RECOVERY_INTERVAL = "network.recovery.interval";
Copy link
Contributor

Choose a reason for hiding this comment

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

connection.recovery.interval?

@michaelklishin michaelklishin merged commit 34cce9b into 4.x.x-stable Nov 10, 2017
@gerhard gerhard deleted the rabbitmq-java-client-324 branch November 10, 2017 15:25
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.

2 participants