After more carefully reading the code and unit tests, I have come to the
conclusion that the interaction between context-specific listeners and the
StatusLogger lifecycle need to be rethought. My approach, at best, would
have arrived at a kludge that worked but made the code difficult to
maintain.
As exhibit A, I point you to the method
configureExistingStatusConsoleListener in StatusConfiguration:
private boolean configureExistingStatusConsoleListener() {
boolean configured = false;
for (final StatusListener statusListener :
this.logger.getListeners()) {
if (statusListener instanceof StatusConsoleListener) {
final StatusConsoleListener listener =
(StatusConsoleListener) statusListener;
listener.setLevel(this.status);
this.logger.updateListenerLevel(this.status);
if (this.verbosity == Verbosity.QUIET) {
listener.setFilters(this.verboseClasses);
}
configured = true;
}
}
return configured;
}
If the code reconfigures existing StatusConsoleListeners when starting up,
then we can't very well shut them down when shutting down a context unless
all contexts that rely on that StatusLogger are shutting down. So my idea
that I would add the context name to each of the listeners and shut down
the correct one is flawed because the listener is likely to be shared
between contexts. Based on this, I have closed the pull request.
I need to take the time to wrap my head around possible lifecycles for the
StatusListener and StatusConsoleListener objects. I think the LoggerContext
needs to manage the context-specific StatusLoggers which means the
StatusConfiguration will need to pass any StatusLogger objects to the
LoggerContext so it can manage them. As I said before, it is a bit of an
X-Y problem: I'm trying to fix the file locking but really we need to
rethink the StatusLogger lifecycle.
I would like to leave my code accessible to you all. Will it remain in the
closed pull request if I change the underlying branch in GitHub? If not, I
can create an "bad_ideas" branch and put the code there.
I will carve out the changes I made to
log4j-kafka\src\test\java\org\apache\logging\log4j\kafka\builder\ConfigurationBuilderTest.java
to make it platform agnostic and create a pull request for that.
I could also create a pull request for the fixes I had to quell warnings if
you all are interested. They just deprecated imports and usage of
constructors for Integer, Long, and Short rather than using the getInteger,
getLong and getShort static methods.
Please give me your thoughts on:
1) managing the StatusLogger lifecycle
2) do I need to move my commits from pull 469 to a branch so they remain
accessible?
3) do you want a pull request for the warnings I fixed?
Thanks,
Tim
On Wed, Apr 28, 2021 at 9:06 AM Tim Perry <tim.v...@gmail.com> wrote:
> I pushed 46f7b08288e1f8843d293bb75950a8b466854ba6 to pull 469.
>
> My recollection is that there was a test failing where I thought the
> problem was the test, not the code. The test was checking to see if the log
> file was gone inside the @Test and I don't think that is valid. I was
> meaning to ask someone if the test was incorrect or if my code was the
> problem.
>
> I'll try to get back to this, but I'm slammed with work at the moment.
>
>
> On Wed, Apr 28, 2021 at 8:42 AM Ralph Goers <ralph.go...@dslextreme.com>
> wrote:
>
>> That would be up to you. If you have been working on the same branch
>> there shouldn’t be a problem. But if it is going to cause merge conflicts
>> then I wouldn’t bother as you can just link to the prior PR for the history.
>>
>> Ralph
>>
>> > On Apr 28, 2021, at 7:55 AM, Tim Perry <tim.v...@gmail.com> wrote:
>> >
>> >
>> >
>> > It assumes that the status logger is not a singleton. However, status
>> logger is a singleton and so this is a problem.
>> >
>> > I’ve implemented to changes which tracks the different status logger
>> listeners by the context name. There are still a couple of kinks to work
>> out on that though. If you want, I can push my changes to GitHub and you
>> could take a look at those. There is some problem that’s causing some of
>> the test to fail and I haven’t had time to figure out what the issue is.
>> >
>> > Tim
>> >
>> >
>> > Tim Perry
>> > (916) 505-3634
>>
>>
