You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by "yybmion (via GitHub)" <gi...@apache.org> on 2025/07/21 09:30:31 UTC
[PR] Add API to list supported configuration file extensions [logging-log4j2]
yybmion opened a new pull request, #3839:
URL: https://github.com/apache/logging-log4j2/pull/3839
### Summary
Add ConfigurationFactory.getActiveFileExtensions() and getSupportedExtensions() methods to allow external frameworks like Spring Boot to determine which configuration file formats are currently supported at runtime.
- Add abstract getSupportedExtensions() method to ConfigurationFactory
- Add static getActiveFileExtensions() method to aggregate results
- Implement getSupportedExtensions() in all ConfigurationFactory subclasses
- Add test coverage for new functionality
### Example Usage
```java
List<String> supportedExtensions = ConfigurationFactory.getActiveFileExtensions();
// Returns: ["xml", "json", "jsn", "properties"] (if Jackson available)
// Returns: ["xml", "properties"] (if Jackson missing)
```
## Checklist
Before we can review and merge your changes, please go through the checklist below. If you're still working on some items, feel free to submit your pull request as a draft—our CI will help guide you through the remaining steps.
### ✅ Required checks
- [x] **License**: I confirm that my changes are submitted under the [Apache License, Version 2.0](https://apache.org/licenses/LICENSE-2.0).
- [x] **Commit signatures**: All commits are signed and verifiable. (See [GitHub Docs on Commit Signature Verification](https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification)).
- [x] **Code formatting**: The code is formatted according to the project’s style guide.
<details>
<summary>How to check and fix formatting</summary>
- To **check** formatting: `./mvnw spotless:check`
- To **fix** formatting: `./mvnw spotless:apply`
See [the build instructions](https://logging.apache.org/log4j/2.x/development.html#building) for details.
</details>
- [x] **Build & Test**: I verified that the project builds and all unit tests pass.
<details>
<summary>How to build the project</summary>
Run: `./mvnw verify`
See [the build instructions](https://logging.apache.org/log4j/2.x/development.html#building) for details.
</details>
### 🧪 Tests (select one)
- [x] I have added or updated tests to cover my changes.
- [ ] No additional tests are needed for this change.
### 📝 Changelog (select one)
- [x] I added a changelog entry in `src/changelog/.2.x.x`. (See [Changelog Entry File Guide](https://logging.apache.org/log4j/tools/log4j-changelog.html#changelog-entry-file)).
- [ ] This is a trivial change and does not require a changelog entry.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add API to list supported configuration file extensions [logging-log4j2]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #3839:
URL: https://github.com/apache/logging-log4j2/pull/3839#discussion_r2288444366
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationFactory.java:
##########
@@ -263,6 +264,14 @@ public static void removeConfigurationFactory(final ConfigurationFactory factory
protected abstract String[] getSupportedTypes();
+ /**
+ * Returns the file extensions supported by this configuration factory.
+ *
+ * @return list of supported file extensions (e.g., ["xml", "json"])
+ * @since 2.25.0
+ */
+ public abstract List<String> getSupportedExtensions();
Review Comment:
`getSupportedFileExtensions` sounds good, although I would keep this a list to convey the order in which those extensions are checked.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add API to list supported configuration file extensions [logging-log4j2]
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on PR #3839:
URL: https://github.com/apache/logging-log4j2/pull/3839#issuecomment-3209220815
> > 1. It duplicates quite some logic (e.g., in `JsonConfigurationFactory`, extensions and suffixes are semantically identical).
>
> Totally agree. Let’s put a **default** implementation in `ConfigurationFactory` so most factories don’t have to repeat themselves.
Please don't – you will still end up having two methods doing the same thing.
> I prefer the dedicated `getSupportedExtensions()` API for a few reasons:
>
> * `isActive()` and `getSupportedTypes()` are **protected**, not public, so they’re not available to consumers; exposing them (or a raw `getFactories()`) would leak internal details.
Agreed. Let's make `isActive()` public.
> * Implementations are **inconsistent** today: some factories always report active but conditionally restrict types (e.g., Log4j 1 XML returns `xml` only when a property is set), while others return a constant type set but flip `isActive()` based on optional deps. A single public method lets us **normalize** that behavior and present a stable contract.
What you're describing is not a problem, but a behavior and it is all good, AFAICT. It is up to the `CF` what to return from `isActive()`. It makes sense `CF.getFactories()` filtered on `isActive()` yields a different outcome when a user has enabled/disabled Log4j 1 support. As a matter of fact, this is a powerful feature that allows `CF`s to dynamically determine their active status.
> * In Log4j 3, `isActive()` isn’t needed anymore: optional Maven **modules** provide support for additional formats, not per-factory optional dependencies, so I would propose to remove `isActive()` from Log4j 3.
That is a good idea.
> * Conceptually, `ConfigurationFactory` should look like a **singleton service**; how it multiplexes concrete factories is an implementation detail. Exposing `getFactories()` would couple users to that detail, whereas `getSupportedExtensions()` gives them exactly what they need without exposing internals.
`CF::getSupportedFileExtensions` _only_ hoists `CF::getSupportedTypes` for a handful of users. OTOH, `CF::instances` would enable several more use cases with less API impact.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add API to list supported configuration file extensions [logging-log4j2]
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on PR #3839:
URL: https://github.com/apache/logging-log4j2/pull/3839#issuecomment-3209253360
Another concern I have regarding adding a new, as a matter of fact, 2nd after `getSupportedTypes()`, _"supported file extensions"_ concept to `CF` is that it reinforces the idea of a `CF` must be associated with a _file_, whereas it actually is not necessarily.
On Tuesday, in a meeting where @ppkarwasz and I were present, after hearing that one of the biggest Log4j users using programmatic configuration extensively, I am very reluctant to marry the concept of files with configurations.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add API to list supported configuration file extensions [logging-log4j2]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on PR #3839:
URL: https://github.com/apache/logging-log4j2/pull/3839#issuecomment-3219871039
My idea for the proposed method
```java
public Configuration getConfiguration(LoggerContext context, String name, List<URI> uris)
```
is to encapsulate the logic currently in `ConfigurationFactory.Factory`:
https://github.com/apache/logging-log4j2/blob/769b9249176b69ad4474fc4bd831dec67775c953/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationFactory.java#L381-L402
This would allow each `ConfigurationFactory` to support building a composite configuration from multiple URIs and will allow Spring Boot to specify the URI of the overrides, without any knowledge of Log4j's composite configuration.
One open question is how to define the nullability/return contract. The existing single-URI overload,
```java
public Configuration getConfiguration(LoggerContext context, String name, URI uri)
```
has inconsistent behavior across implementations:
* Individual `ConfigurationFactory` implementations typically return `null` if the URI itself is `null`.
* `ConfigurationFactory.Factory`, however, falls back to a `DefaultConfiguration`.
Most (but not all) callers explicitly check for `null`, so the contract is somewhat unclear. For the new multi-URI variant, we should decide whether the method is allowed to return `null`, or whether it must always return a valid `Configuration` (possibly default).
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add API to list supported configuration file extensions [logging-log4j2]
Posted by "yybmion (via GitHub)" <gi...@apache.org>.
yybmion commented on PR #3839:
URL: https://github.com/apache/logging-log4j2/pull/3839#issuecomment-3265754355
@vy @ppkarwasz Thanks for the detailed guidance.
I Created a new PR based on this discussion: #3921
Implemented the suggested approach
- New method: `getConfiguration(LoggerContext, String, List<URI>)`
- Encapsulates composite configuration creation
- Throws exceptions instead of returning null
- Minimal changes, no refactoring (as advised)
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add API to list supported configuration file extensions [logging-log4j2]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on PR #3839:
URL: https://github.com/apache/logging-log4j2/pull/3839#issuecomment-3206986816
> 1. It duplicates quite some logic (e.g., in `JsonConfigurationFactory`, extensions and suffixes are semantically identical).
Totally agree. Let’s put a **default** implementation in `ConfigurationFactory` so most factories don’t have to repeat themselves. Something like:
```java
@Override
public List<String> getSupportedExtensions() {
final String[] types = getSupportedTypes(); // protected, legacy API
// Preserve legacy semantics: only advertise extensions when active and types are present.
return isActive() && types != null && types.length > 0
? Arrays.asList(types)
: Collections.emptyList();
}
```
> 2. Supported configuration file extensions *could* be obtained from `ConfigurationFactory.getFactories()` by filtering on `isActive()` and collecting `getSupportedTypes()` *if* only `getFactories()` would have existed. Put another way, why don't we add a static `getFactories()` to `CF` and be done with it?
I prefer the dedicated `getSupportedExtensions()` API for a few reasons:
* `isActive()` and `getSupportedTypes()` are **protected**, not public, so they’re not available to consumers; exposing them (or a raw `getFactories()`) would leak internal details.
* Implementations are **inconsistent** today: some factories always report active but conditionally restrict types (e.g., Log4j 1 XML returns `xml` only when a property is set), while others return a constant type set but flip `isActive()` based on optional deps. A single public method lets us **normalize** that behavior and present a stable contract.
* In Log4j 3, `isActive()` isn’t needed anymore: optional Maven **modules** provide support for additional formats, not per-factory optional dependencies, so I would propose to remove `isActive()` from Log4j 3.
* Conceptually, `ConfigurationFactory` should look like a **singleton service**; how it multiplexes concrete factories is an implementation detail. Exposing `getFactories()` would couple users to that detail, whereas `getSupportedExtensions()` gives them exactly what they need without exposing internals.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add API to list supported configuration file extensions [logging-log4j2]
Posted by "yybmion (via GitHub)" <gi...@apache.org>.
yybmion commented on PR #3839:
URL: https://github.com/apache/logging-log4j2/pull/3839#issuecomment-3201841069
Hi @ppkarwasz , Just a gentle reminder — PTAL when you have time.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add API to list supported configuration file extensions [logging-log4j2]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on PR #3839:
URL: https://github.com/apache/logging-log4j2/pull/3839#issuecomment-3211804379
> > * Implementations are **inconsistent** today: some factories always report active but conditionally restrict types (e.g., Log4j 1 XML returns `xml` only when a property is set), while others return a constant type set but flip `isActive()` based on optional deps. A single public method lets us **normalize** that behavior and present a stable contract.
>
> What you're describing is not a problem, but a behavior and it is all good, AFAICT. It is up to the `CF` what to return from `isActive()`. It makes sense `CF.getFactories()` filtered on `isActive()` yields a different outcome when a user has enabled/disabled Log4j 1 support. As a matter of fact, this is a powerful feature that allows `CF`s to dynamically determine their active status.
The inconsistency is in the way a factory communicates that it is inactive:
- If [`o.a.log4j.config.PropertiesConfigurationFactory`](https://github.com/apache/logging-log4j2/blob/2.x/log4j-1.2-api/src/main/java/org/apache/log4j/config/PropertiesConfigurationFactory.java) is **not** active, then `isActive()` return `true`, but `getSupportedTypes()` returns `null`.
- On the other hand if [`YamlConfigurationFactory`](https://github.com/apache/logging-log4j2/blob/2.x/log4j-core/src/main/java/org/apache/logging/log4j/core/config/yaml/YamlConfigurationFactory.java) is **not** active, then `isActive()` returns `false`, while `getSupportedTypes()` returns `[".yml", ".yaml"]`.
My point is that, as they are now implemented, it does **not** make sense to make these methods public.
> Another concern I have regarding adding a new, as a matter of fact, 2nd after `getSupportedTypes()`, _"supported file extensions"_ concept to `CF` is that it reinforces the idea of a `CF` must be associated with a _file_, whereas it actually is not necessarily.
I agree completely: a `ConfigurationFactory` **should not** be inherently tied to a file. Unfortunately, the current discovery mechanism assumes the presence of a file resource. A good example is [`SpringBootConfigurationFactory`](https://github.com/spring-projects/spring-boot/blob/main/core/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/SpringBootConfigurationFactory.java), which only works if a dummy `log4j2.springboot` file is present.
> > * `isActive()` and `getSupportedTypes()` are **protected**, not public, so they’re not available to consumers; exposing them (or a raw `getFactories()`) would leak internal details.
>
> Agreed. Let's make `isActive()` public.
> [...]
> > * In Log4j 3, `isActive()` isn’t needed anymore: optional Maven **modules** provide support for additional formats, not per-factory optional dependencies, so I would propose to remove `isActive()` from Log4j 3.
>
> That is a good idea.
I don’t think we should make either `isActive()` or `getSupportedTypes()` public.
* **`isActive()`**: aside from its inconsistent semantics today (which we could technically fix), we’ve already agreed it should disappear in Log4j Core 3 where the concept no longer applies. Therefore we can not propose its usage to Spring Boot if we want them to handle both Log4j Core 2 and 3.
* **`getSupportedTypes()`**: as you noted, this reinforces the idea that every `ConfigurationFactory` must be file-based, which is not always true. Exposing it publicly would just entrench that misconception further.
### Alternative proposal
Looking again at [`Log4j2LoggingSystem`](https://github.com/spring-projects/spring-boot/blob/main/core/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystem.java), I realized that the `getSupportedExtensions` method I suggested is really addressing an *XY problem*: I proposed expanding our API surface only to accommodate how Spring Boot **currently** interacts with `ConfigurationFactory`.
If we take a step back, Spring Boot doesn’t actually need to know about multiple factories, or how we handle extensions and precedence. What it needs boils down to:
1. **User-specified config (`logging.config`, plus optional `logging.log4j2.config.override`):**
Spring Boot should resolve those locations to `URL`s and call `ConfigurationFactory.getConfiguration(LoggerContext, String, URI)`.
If overrides are provided, they need a way to combine them. That suggests introducing a dedicated method like `getConfiguration(LoggerContext, String, List<URI>)` so they don't need to touch our “private” API.
2. **No explicit config provided:**
Today, Spring Boot checks for “standard” configuration files. In reality, they could just verify whether Log4j Core has already been initialized with a non-default configuration:
```java
!config.getConfigurationSource().equals(ConfigurationSource.NULL)
```
If it’s still the default config, they know they should look for alternatives.
3. **Spring-specific defaults (e.g., `log4j2-spring.xml`):**
This is where they currently rely on extensions again. But they could instead let Log4j Core resolve it via the `name` parameter:
```java
ConfigurationFactory.getInstance().getConfiguration(ctx, "-spring", null);
```
4. **Fallback:**
If all of the above fail, they fall back to their standard configuration file.
**TL;DR:**
Spring Boot does not actually need to know what extensions we support. With the existing `ConfigurationFactory.getConfiguration` API (plus potentially a small addition for composite configs), they can cover all their use cases. The only real gap is an ergonomic way to build composite configurations; everything else can stay encapsulated inside Log4j Core.
@yybmion, apologies that your PR turned into a broader discussion about `ConfigurationFactory`’s shortcomings. That said, having this concrete example was valuable: it helped us see that the path we were considering (exposing supported extensions) may not be the right one after all.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add API to list supported configuration file extensions [logging-log4j2]
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on PR #3839:
URL: https://github.com/apache/logging-log4j2/pull/3839#issuecomment-3225044795
> One open question is how to define the nullability/return contract. The existing single-URI overload,
>
> ```java
> public Configuration getConfiguration(LoggerContext context, String name, URI uri)
> ```
>
> has inconsistent behavior across implementations:
>
> * Individual `ConfigurationFactory` implementations typically return `null` if the URI itself is `null`.
> * `ConfigurationFactory.Factory`, however, falls back to a `DefaultConfiguration`.
>
> Most (but not all) callers explicitly check for `null`, so the contract is somewhat unclear. For the new multi-URI variant, we should decide whether the method is allowed to return `null`, or whether it must always return a valid `Configuration` (possibly default).
@ppkarwasz, since there is no established default that one can claim as a stable API, I'm inclined to throw `IAE`, or `NPE` (when arg is null) on failures. WDYT?
Will this new API necessitate an amendment/improvement to the manual? (I've skimmed through combined/composite configuration sections, but couldn't see any.)
> Should I start working on this right away, or would you prefer to clarify a few implementation details first (like the nullability contract and specific behavior expectations)?
@yybmion, please go ahead. I strongly advise you to work out the essentials first, and leave out the tests, docs, etc. Once we get it reviewed, you can continue with polishing the PR.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add API to list supported configuration file extensions [logging-log4j2]
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on PR #3839:
URL: https://github.com/apache/logging-log4j2/pull/3839#issuecomment-3206278760
@ppkarwasz, would you mind reminding me why we decided to introduce a new API such that
1. It duplicates quite some logic (e.g., in `JsonConfigurationFactory`, extensions and suffixes are semantically identical)
2. Supported configuration file extensions _could_ be obtained from `ConfigurationFactory.getFactories()` by filtering on `isActive()` and collecting `getSupportedTypes()` *if* only `getFactories()` would have existed
Put another way, why don't we add a static `getFactories()` to `CF` and be done with it?
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add API to list supported configuration file extensions [logging-log4j2]
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on PR #3839:
URL: https://github.com/apache/logging-log4j2/pull/3839#issuecomment-3262159881
@ppkarwasz, can you explain why would we want to return null instead of throwing? Note that _"because we sometimes return null"_ does not constitute as an argument, since what we're talking about is a new API.
> That mix of `null` vs `DefaultConfiguration` is problematic: users will almost always guard for one but not the other. For example, Spring Boot’s `Log4j2LoggingSystem` never checks for `null`.
> ...
Using this argument...
> * **Return contract:** always `null` on failure (with legacy `DefaultConfiguration` as deprecated).
... you're proposing making _"returning `null` on failure"_ the contract. I can use the very same argument to propose the opposite: _"throwing on failure"_. Note that [PMC had consensus for throwing on configuration failures](https://lists.apache.org/thread/h2oydyk6xld47ljttqvflbt4530o73vw). Hence, I find your proposal contradictory. I'm inclined to throw on failures.
> * **Parameter contract:** permissive for existing args, strict (`NPE`) only for the new `List<URI>`.
I agree with your assessment here. I'd appreciate all these (param. spec.) to be documented in the Javadoc of the new method.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add API to list supported configuration file extensions [logging-log4j2]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on PR #3839:
URL: https://github.com/apache/logging-log4j2/pull/3839#issuecomment-3233181160
> @ppkarwasz, since there is no established default that one can claim as a stable API, I'm inclined to throw `IAE`, or `NPE` (when arg is null) on failures. WDYT?
I think the bigger issue is **the nullability and return behavior**, which is inconsistent across the existing `getConfiguration(...)` methods:
```java
public Configuration getConfiguration(LoggerContext ctx, ConfigurationSource source);
public Configuration getConfiguration(LoggerContext ctx, String name, URI location);
public Configuration getConfiguration(LoggerContext ctx, String name, URI location, ClassLoader loader);
```
* All three can return `null` in some cases.
* But the second (and therefore the third) will return a `DefaultConfiguration` in the most common failure case.
That mix of `null` vs `DefaultConfiguration` is problematic: users will almost always guard for one but not the other. For example, Spring Boot’s `Log4j2LoggingSystem` never checks for `null`.
## My proposal
### Return behavior
* Clearly document that these methods **can and do** return `null`.
* Treat the “fallback to `DefaultConfiguration`” behavior as **deprecated**, with the intent to remove it in a future major release. This gives consumers a single, consistent contract to rely on.
### Parameter nullability
For arguments, I’d keep the current semantics and only be strict where it adds value:
* `LoggerContext`: optional. It’s mostly passed through to rare plugins (interpolator lookups, `LoggerConfig` default `includeLocation`), so it can remain nullable.
* `String name`: optional. Used as a profile hint (`log4j2-spring.xml` vs `log4j2.xml`), so nullable is fine.
* `ConfigurationSource`: should not be optional, but if you pass `null` today, you get `null`—so no need to change that.
* `URI`: optional. It’s just a hint about where the config came from.
* `List<URI>` (new overload): here I’d draw the line. Passing `null` is meaningless when an empty list is a valid “no configs” signal. I’d propose we explicitly throw `NPE` if the list itself is `null`.
This way we converge towards:
* **Return contract:** always `null` on failure (with legacy `DefaultConfiguration` as deprecated).
* **Parameter contract:** permissive for existing args, strict (`NPE`) only for the new `List<URI>`.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add API to list supported configuration file extensions [logging-log4j2]
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy closed pull request #3839: Add API to list supported configuration file extensions
URL: https://github.com/apache/logging-log4j2/pull/3839
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add API to list supported configuration file extensions [logging-log4j2]
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on PR #3839:
URL: https://github.com/apache/logging-log4j2/pull/3839#issuecomment-3219502641
> * **`isActive()`**: aside from its inconsistent semantics today (which we could technically fix)
Created #3896 for this.
> **TL;DR:** Spring Boot does not actually need to know what extensions we support. With the existing `ConfigurationFactory.getConfiguration` API (plus potentially a small addition for composite configs), they can cover all their use cases. The only real gap is an ergonomic way to build composite configurations; everything else can stay encapsulated inside Log4j Core.
I'm happy to see that the nudge by @yybmion helped us to avoid yet another [peak of complexity](https://www.youtube.com/watch?v=Yiye8lqh0Ig). @ppkarwasz, thanks so much for your patience along this conversation and helping with evaluating alternatives. I liked your proposal on changing `CF::gC` as you suggested in the paragraph I quoted. Would you mind sharing more details such that @yybmion can flesh it out further in a PR<sup>1</sup>?
<sup>1</sup> I'm inclined to leave this PR as is for records, and implement the new `CF::gC` change in a separate PR.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add API to list supported configuration file extensions [logging-log4j2]
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on PR #3839:
URL: https://github.com/apache/logging-log4j2/pull/3839#issuecomment-3415189120
Closing in favor of #3921.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add API to list supported configuration file extensions [logging-log4j2]
Posted by "yybmion (via GitHub)" <gi...@apache.org>.
yybmion commented on code in PR #3839:
URL: https://github.com/apache/logging-log4j2/pull/3839#discussion_r2289835030
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationFactory.java:
##########
@@ -617,4 +636,19 @@ private String[] parseConfigLocations(final String configLocations) {
static List<ConfigurationFactory> getFactories() {
return factories;
}
+
+ /**
+ * Returns all configuration file extensions currently supported at runtime.
+ * This aggregates extensions from all active configuration factories.
+ *
+ * @return list of supported file extensions
+ * @since 2.25.0
+ */
+ public static List<String> getActiveFileExtensions() {
Review Comment:
Thanks for the review @vy !
You're absolutely right about the redundancy. The static method duplicates the functionality already provided by the instance method.
Therefore, I think it would be better to remove the static `getActiveFileExtensions()` method and use the instance method approach instead.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add API to list supported configuration file extensions [logging-log4j2]
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on code in PR #3839:
URL: https://github.com/apache/logging-log4j2/pull/3839#discussion_r2288091615
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationFactory.java:
##########
@@ -617,4 +636,19 @@ private String[] parseConfigLocations(final String configLocations) {
static List<ConfigurationFactory> getFactories() {
return factories;
}
+
+ /**
+ * Returns all configuration file extensions currently supported at runtime.
+ * This aggregates extensions from all active configuration factories.
+ *
+ * @return list of supported file extensions
+ * @since 2.25.0
+ */
+ public static List<String> getActiveFileExtensions() {
Review Comment:
What is the use case for this? I have the impression the instance method would just do fine.
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationFactory.java:
##########
@@ -263,6 +264,14 @@ public static void removeConfigurationFactory(final ConfigurationFactory factory
protected abstract String[] getSupportedTypes();
+ /**
+ * Returns the file extensions supported by this configuration factory.
+ *
+ * @return list of supported file extensions (e.g., ["xml", "json"])
+ * @since 2.25.0
+ */
+ public abstract List<String> getSupportedExtensions();
Review Comment:
1. Shall we use `Set<String>` instead?
2. Shall we rename this to `getSupportedFileExtensions` to emphasize the _file_ aspect?
3. Version will probably need to be bumped to `2.26.0`
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add API to list supported configuration file extensions [logging-log4j2]
Posted by "yybmion (via GitHub)" <gi...@apache.org>.
yybmion commented on PR #3839:
URL: https://github.com/apache/logging-log4j2/pull/3839#issuecomment-3223735901
@ppkarwasz @vy Thank you both for the incredibly thoughtful discussion and analysis!
I really appreciate how this conversation evolved from my initial approach to a much deeper understanding of the underlying problem.
Should I start working on this right away, or would you prefer to clarify a few implementation details first (like the nullability contract and specific behavior expectations)?
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add API to list supported configuration file extensions [logging-log4j2]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #3839:
URL: https://github.com/apache/logging-log4j2/pull/3839#issuecomment-3099366989
<table>
<tr>
<th>Job</th>
<th>Requested goals</th>
<th>Build Tool Version</th>
<th>Build Outcome</th>
<th>Build Scan®</th>
</tr>
<tr>
<td>build-macos-latest</td>
<td>clean install</td>
<td align='center'>3.9.8</td>
<td align='center'>:white_check_mark:</td>
<td><a href="https://develocity.apache.org/s/qkkbb3qvrfxeq" rel="nofollow"><img src="https://img.shields.io/badge/Build%20Scan%C2%AE-PUBLISHED-06A0CE?logo=Gradle" alt="Build Scan PUBLISHED" /></a></td>
</tr>
<tr>
<td>build-ubuntu-latest</td>
<td>clean install</td>
<td align='center'>3.9.8</td>
<td align='center'>:white_check_mark:</td>
<td><a href="https://develocity.apache.org/s/spfehzlr3vhko" rel="nofollow"><img src="https://img.shields.io/badge/Build%20Scan%C2%AE-PUBLISHED-06A0CE?logo=Gradle" alt="Build Scan PUBLISHED" /></a></td>
</tr>
<tr>
<td>build-windows-latest</td>
<td>clean install</td>
<td align='center'>3.9.8</td>
<td align='center'>:white_check_mark:</td>
<td><a href="https://develocity.apache.org/s/4dshbr2kmjbni" rel="nofollow"><img src="https://img.shields.io/badge/Build%20Scan%C2%AE-PUBLISHED-06A0CE?logo=Gradle" alt="Build Scan PUBLISHED" /></a></td>
</tr>
</table>
###### Generated by gradle/develocity-actions
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org