Skip to content

Update logging#62

Closed
DomGarguilo wants to merge 1 commit intoapache:mainfrom
DomGarguilo:log4jUpdate
Closed

Update logging#62
DomGarguilo wants to merge 1 commit intoapache:mainfrom
DomGarguilo:log4jUpdate

Conversation

@DomGarguilo
Copy link
Member

This PR updates the logging in this repo.

Things seem to work correctly for the most part although for some reason, only the log.error statements are being printed during the tests. This might be caused by some config issue but I am not sure.

I based these changes off of some of the other repositories dependencies but I am not sure that everything is correct or optimal.

@DomGarguilo DomGarguilo requested a review from ctubbsii January 13, 2023 19:06
@DomGarguilo DomGarguilo linked an issue Jan 13, 2023 that may be closed by this pull request
@dlmarion
Copy link
Contributor

We removed SLF4J?

@DomGarguilo
Copy link
Member Author

We removed SLF4J?

Oops, maybe not. I'll add it back.

@DomGarguilo
Copy link
Member Author

We removed SLF4J?

So actually the slf4j dependencies where removed from the pom because the log4j-slf4j2-impl provides an slf4j "provider". If the org.slf4j dependencies are present, a warning is thrown.

@EdColeman
Copy link
Contributor

In the main accumulo pom - log4j-slf4j2-impl is declared as unused dependency. For proxy, would runtime or provided scope be more appropriate and resolve the build error? The real dependency is slf4j - the implementation(s) provided by log4j2 could also be satisfied with logback (I think, it has been I while since I have used that instead of log4j2) so is the -impl is transient and needs to be provided at runtime (and for tests?) but that is a runtime detail and not a dependency in maven build terms.

@ctubbsii
Copy link
Member

Superseded by my fix to logging in 7bcf98a

@ctubbsii ctubbsii closed this Jan 18, 2023
@ctubbsii ctubbsii removed their request for review January 18, 2023 03:55
@DomGarguilo DomGarguilo deleted the log4jUpdate branch January 18, 2023 13:55
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.

Logging does not work

4 participants

Comments