vy commented on PR #1401:
URL: https://github.com/apache/logging-log4j2/pull/1401#issuecomment-1547545535
Thanks for taking time to share your feedback @rgoers, much appreciated!
:bow:
> I was actually looking for this today as I will be needing to add another
ThreadLocal (as much as I really don't want to). But in looking at this PR
again I do have some concerns about it.
>
> 1. It uses a Factory to provide a "specification" (really just a String
key) to determine what kind of Recycler to provide. That seems somewhat ugly to
me. I'd prefer a Spec object that provides attributes for what the
characteristics are.
`RecyclerFactories.ofSpec(String)` static factory method is there to only
parse the textual recycler factory specification read from a user- provided
property. Components will be automatically provided the global [user-set]
`RecyclerFactory` in their `Configuration` instance. For those which need to
~run~ test against a particular RF implementation, they can directly reference
them; e.g., `ThreadLocalRecyclerFactory::new`.
> 2. It seems every ThreadLocal usage will simply be replaced by a Recycler
instance. Instead, I would prefer that there is a Recylcer registry (i.e. - a
Map) that contains all the Recyclers. That allows us to reference a Recycler
from anywhere without needing them to be injected or passed around.
I assume you mean a static registry as in
`RecyclerFactoryRegistry.getDefaultRecyclerFactory()` – please correct me
otherwise.
1. Static registries are proven to be a big problem both at runtime and
[parallel] tests.
1. When used at runtime, they make it impossible to provide different
values for different components. Something conflicts with the DI principles.
1. When used for tests, they cannot be mocked and/or changed per test.
This is one of the main reasons we need to fork a new JVM for almost all tests
currently.
1. I am not able to see the benefit a global registry. Components should be
agnostic to the provided implementation passed via
`Configuration#getRecyclerFactory()`. Mind sharing some uses cases, please?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
