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

Use hazelcast.yaml when hazelcast.xml is not available #20003

Conversation

frant-hartm
Copy link
Contributor

@frant-hartm frant-hartm commented Dec 1, 2021

Users sometimes prefer to use yaml configuration.
When the hazelcast.xml is not available in the config folder then
hazelcast.yaml is tried.
If hazelcast.yaml is not available error message is shown with guidance
what to do.

Fixes #19727

Checklist:

  • Labels (Team:, Type:, Source:, Module:) and Milestone set
  • Label Add to Release Notes or Not Release Notes content set
  • Request reviewers if possible
  • Send backports/forwardports if fix needs to be applied to past/future releases

Users sometimes prefer to use yaml configuration.
When the hazelcast.xml is not available in the config folder then
hazelcast.yaml is tried.
If hazelcast.yaml is not available error message is shown with guidance
what to do.

Fixes hazelcast#19727
fi

if [ ! -f "$HAZELCAST_HOME/$HAZELCAST_CONFIG" ]; then
echo "Configuration file is missing. Create hazelcast.xml or hazelcast.yaml in $HAZELCAST_HOME/config or set the HAZELCAST_CONFIG environment variable."
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if we should exit. According to https://docs.hazelcast.com/hazelcast/5.0/configuration/checking-configuration we should check for ".yml" as well and if none is present then load the default configuration

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point about the yml file. I will add that.

Otherwise, the doc is for embedded use. I am not sure we want to follow the same procedure for our server distribution. Maybe it should be updated what is done for server instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason why we should have different behavior on server distribution? I could imagine it could be surprising for customers using embedded versions while migrating to the server distribution. Also, it brings additional effort on documentation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it makes sense for the distribution to try the options done for embedded:

  • classpath - do we want to encourage users to put config inside custom jars? I don't think so.
  • working directory - currently that's HAZELCAST_HOME if you run hz-start, but we don't have it documented
  • the default config in hazelcast jar is not suitable for server distribution (e.g. it opens on all interfaces, instead of just loopback)

Copy link
Contributor

Choose a reason for hiding this comment

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

fair enough

@hz-devops-test
Copy link

The job Hazelcast-pr-builder of your PR failed. (Hazelcast internal details: build log, artifacts).
Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log file
--------------------------
-------TEST FAILURE-------
--------------------------
[INFO] Results:
[INFO] 
[ERROR] Failures: 
[ERROR]   Log4jLoggerTest.logEvent_withLogLevelOff_shouldNotLog:131 [Expected 0 log events but got 1] 
Expecting:
 <1>
to be equal to:
 <0>
but was not.
[INFO] 
[ERROR] Tests run: 46543, Failures: 1, Errors: 0, Skipped: 1012
[INFO] 

[ERROR] There are test failures.

@frant-hartm
Copy link
Contributor Author

run-lab-run

Copy link
Member

@kwart kwart left a comment

Choose a reason for hiding this comment

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

Ah, I think we're missing changes in the batch file.

@hz-devops-test
Copy link

The job Hazelcast-pr-builder of your PR failed. (Hazelcast internal details: build log, artifacts).
Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log file
--------------------------
-------TEST FAILURE-------
--------------------------
[INFO] Results:
[INFO] 
[ERROR] Failures: 
[ERROR]   ClientMapPartitionLostListenerTest.test_mapPartitionLostListener_invoked_fromOtherNode:133->assertProxyExistsEventually:163->HazelcastTestSupport.assertTrueEventually:1362->HazelcastTestSupport.assertTrueEventually:1260 There is no proxy with name 5ed8ab7e-acad-4c77-9c69-c500e1e21a47 created (yet)
[INFO] 
[ERROR] Tests run: 46543, Failures: 1, Errors: 0, Skipped: 1012
[INFO] 

[ERROR] There are test failures.

@@ -41,7 +41,16 @@ IF NOT "%JAVA_VERSION%" == "8" (

REM HAZELCAST_CONFIG holds path to the configuration file. The path is relative to the Hazelcast installation (HAZELCAST_HOME).
if "x%HAZELCAST_CONFIG%" == "x" (
set HAZELCAST_CONFIG=config/hazelcast.xml
Copy link
Member

Choose a reason for hiding this comment

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

I think we want to keep this line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that was a mistake.

Copy link
Contributor Author

@frant-hartm frant-hartm Dec 1, 2021

Choose a reason for hiding this comment

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

I changed the slash though, so it still shows as deleted, but it's added below.

@frant-hartm frant-hartm merged commit 6750b72 into hazelcast:master Dec 2, 2021
frant-hartm added a commit that referenced this pull request Dec 2, 2021
* Allow absolute paths in HAZELCAST_CONFIG environment variable (#19908)

Allow absolute paths in HAZELCAST_CONFIG and HAZELCAST_CLIENT_CONFIG environment variables

* Use hazelcast.yaml when hazelcast.xml is not available

Users sometimes prefer to use yaml configuration.
When the hazelcast.xml is not available in the config folder then
hazelcast.yaml is tried.
If hazelcast.yaml is not available error message is shown with guidance
what to do.

Backport of #20003 and #19908 (for clean backport)

Fixes #19727

Co-authored-by: Yüce Tekol <yucetekol@gmail.com>
sancar pushed a commit to sancar/hazelcast that referenced this pull request Dec 13, 2021
* Use hazelcast.yaml when hazelcast.xml is not available

Users sometimes prefer to use yaml configuration.
When the hazelcast.xml is not available in the config folder then
hazelcast.yaml is tried.
If hazelcast.yaml is not available error message is shown with guidance
what to do.

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

Successfully merging this pull request may close these issues.

YAML configuration file not automatically used
4 participants