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

Load conf/log4j.properties automatically if exists #203

Conversation

xabolcs
Copy link
Collaborator

@xabolcs xabolcs commented Jun 27, 2023

Hi @asolntsev ,

this is another low hanging fruit: let the Framework load the log4j.properties from conf directory if exists.
I also added a failing test before implementing the improvement.

xabolcs added 2 commits June 28, 2023 01:26
In Play! 1.5.x documentation [1] the property "application.log.path"
description is the following:
Path to a Log4J configuration file, to customise log output.
If you do not specify a path, Play will load a log4j.properties file
in the conf directory if present.

The same can be found in the annotated application.conf:
"If you want a very customized log, create a log4j.properties file
in the conf directory"

I'm unsure if it worked at all in Play!, but it's a nice feature.

The init_with_conf_dir() test tries to check the parameters of the newly
added "conf/log4j.properties".

The init_with_default_config() test deletes that properties file
from the test classpath and checks for the Framework's properties file.

[1]: https://www.playframework.com/documentation/1.5.x/configuration#application.log.path
In Play! 1.5.x documentation [1] the property "application.log.path"
description is the following:
Path to a Log4J configuration file, to customise log output.
If you do not specify a path, Play will load a log4j.properties file
in the conf directory if present.

The same can be found in the annotated application.conf:
"If you want a very customized log, create a log4j.properties file
in the conf directory"

I'm unsure if it worked at all in Play!, but it's a nice feature.

[1]: https://www.playframework.com/documentation/1.5.x/configuration#application.log.path
@asolntsev
Copy link
Contributor

@xabolcs Wait, but... I thought RePlay adds "conf" directory to classpath anyway.
Thus searching resource "/log4j.properties" WILL find the file "conf/log4j.properties".

@xabolcs
Copy link
Collaborator Author

xabolcs commented Aug 9, 2023

Well, that's why I added the failing test. 🙂

... And strictly speaking: RePlay doesn't add the conf directory to the classpath, but the packaging software (Maven, Gradle), and they copy only it's contents to the root directory if the jar. (Yeah this is a simplification, as Maven and Gradle does only what the user - the RePlay App developer - wants 🙂)

This PR improves the situation for those who copy the conf dir as-is into the jar / classpath: when the log4j.properties file is in the conf/ directory inside the jar (or in the classpath)

@asolntsev asolntsev self-assigned this Aug 9, 2023
@asolntsev asolntsev added this to the 2.2.0 milestone Aug 9, 2023
@asolntsev
Copy link
Contributor

@xabolcs Ah so. Thanks, it explains why you need it. Yes, I think in our working projects we added conf folder as "source root", thus its content was copied by Gradle to the root of classpath.

@asolntsev asolntsev merged commit 112147a into replay-framework:main Aug 9, 2023
@asolntsev asolntsev self-requested a review August 9, 2023 06:33
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