Skip to content

Remove non-functional variable definition in log4j.properties#36

Closed
rocketraman wants to merge 1 commit intoapache:0.8.1from
rocketraman:patch-1
Closed

Remove non-functional variable definition in log4j.properties#36
rocketraman wants to merge 1 commit intoapache:0.8.1from
rocketraman:patch-1

Conversation

@rocketraman
Copy link
Copy Markdown
Member

In log4j.properties, a property kafka.logs.dir was defined. However, modifying this property has no effect because log4j.properties does not support variable substitution in this manner. Rather ${kafka.logs.dir} is looked up as a System property, which is set in bin/kafka-run-class.sh, and must currently be changed there if the log4j logging location is to be modified.

In log4j.properties, a property kafka.logs.dir was defined. However, modifying this property has no effect because log4j.properties does not support variable substitution in this manner. Rather ${kafka.logs.dir} is looked up as a System property, which is set in bin/kafka-run-class.sh, and must currently be changed there if the log4j logging location is to be modified.
@junrao
Copy link
Copy Markdown
Contributor

junrao commented Nov 26, 2014

It seems kafka.logs.dir is just local variable for local reference.

@rocketraman
Copy link
Copy Markdown
Member Author

It seems kafka.logs.dir is just local variable for local reference.

It isn't though. Try changing its value. You will see that nothing actually changes i.e. the value of the kafka.logs.dir system property is used instead. So having it there is highly misleading (I was bitten by this).

@rocketraman
Copy link
Copy Markdown
Member Author

A little bit more information: according to http://logging.apache.org/log4j/1.2/apidocs/org/apache/log4j/PropertyConfigurator.html the property is first searched in the system properties, and then only if the system property does not exist, then in the local file (which is weird, I would have implemented it in the reverse search order, to allow a system default with a local override).

Since kafka.logs.dir is always created as a system property by the start script, the local property in log4j.properties is never used.

@junrao
Copy link
Copy Markdown
Contributor

junrao commented Nov 26, 2014

Thanks for the explanation. Could you create a Kafka jira and attach a patch there? This will take care of the Apache licensing stuff.

@rocketraman
Copy link
Copy Markdown
Member Author

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Jul 20, 2015

Can you please close this pull request since the issue has been solved (we can't do it ourselves without asking Apache Infra via a ticket).

@rocketraman
Copy link
Copy Markdown
Member Author

Done!

guozhangwang referenced this pull request in confluentinc/kafka Aug 5, 2015
Refactor state store API with some fixes on restoration logic.
ymatsuda added a commit to ymatsuda/kafka that referenced this pull request Sep 8, 2015
resetius added a commit to resetius/kafka that referenced this pull request Apr 22, 2016
[LOGBROKER-630] Fix posible races
lianetm pushed a commit to lianetm/kafka that referenced this pull request Aug 8, 2023
Fix the test

Revert the previous changes
rustd pushed a commit to rustd/pranavfinaldemokafka that referenced this pull request Feb 9, 2024
Reviewers: José Armando García Sancio <jsancio@confluent.io>

Co-authored-by: Mickael Maison <mimaison@users.noreply.github.com>
fvaleri pushed a commit to fvaleri/kafka that referenced this pull request Oct 24, 2025
Add remote fetch authentication
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.

3 participants