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

added possibility to set default_advertised_address through environment variable #68

Merged
merged 3 commits into from
Nov 29, 2016

Conversation

unterstein
Copy link
Contributor

@unterstein unterstein commented Nov 25, 2016

Fixes #67

My intention:
In order to support networking features like overlay networks and virtual IPs, it would be great if dbms.connectors.default_advertised_address would be configurable. It would be great, if it would be possible to inject the advertised addressed from the outside, for example using NEO4J_dbms_advertisedAddress environment variable.
This property should only be set, if the environment variable is set.

I compared the neo4j.conf in 2.3 and 3.0 branch and did not found dbms.connectors.default_advertised_address property, therefore I only added this configuration only in 3.1 and 3.2.

@unterstein unterstein changed the title added possibility to set through environment variable added possibility to set default_advertised_address through environment variable Nov 25, 2016
@@ -42,6 +42,9 @@ if [ "$1" == "neo4j" ]; then
setting "dbms.connector.https.listen_address" "0.0.0.0:7473"
setting "dbms.connector.bolt.listen_address" "0.0.0.0:7687"
setting "dbms.mode" "${NEO4J_dbms_mode:-}"
if [ -n "$NEO4J_dbms_advertisedAddress" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

We've started using a convention for future improvements, so this variable should be NEO4J_dbms_connectors_defaultAdvertisedAddress.

Also, by using a default value as on the next line, you should be able to get rid of the if statement like so:

setting "dbms.connectors.default_advertised_address" "${NEO4J_dbms_connectors_defaultAdvertisedAddress:-}"

After you make those changes and test it, I'll be happy to merge it quickly.

Thanks for your contribution!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi,

thanks for your feedback. Is this ok?

Cheers

@srbaker
Copy link
Contributor

srbaker commented Nov 28, 2016

Perfect, thanks! I'll get this merged tomorrow (Europe time).

unterstein added a commit to unterstein/dcos-neo4j that referenced this pull request Nov 29, 2016
@srbaker srbaker merged commit 5ed5a5c into neo4j:master Nov 29, 2016
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.

None yet

2 participants