Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Suppress the StanfordNLP logger #701

Merged
merged 1 commit into from
Sep 29, 2022
Merged

Suppress the StanfordNLP logger #701

merged 1 commit into from
Sep 29, 2022

Conversation

tsaglam
Copy link
Member

@tsaglam tsaglam commented Sep 29, 2022

Hotfix that suppresses the StanfordNLP logger directly, with the exception of errors.

@tsaglam tsaglam added enhancement Issue/PR that involves features, improvements and other changes minor Minor issue/feature/contribution/change labels Sep 29, 2022
@tsaglam tsaglam added this to the v4.0.0 milestone Sep 29, 2022
@tsaglam tsaglam force-pushed the suppress-stanford-logger branch from a055bea to 7a09ffd Compare September 29, 2022 08:32
@tsaglam tsaglam marked this pull request as ready for review September 29, 2022 08:32
@tsaglam tsaglam requested a review from a team September 29, 2022 08:33
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@dfuchss
Copy link
Member

dfuchss commented Sep 29, 2022

Log levels should not be managed in the client code (from my point of view). This should be done in the logger config of the CLI.

@dfuchss
Copy link
Member

dfuchss commented Sep 29, 2022

See https://github.com/ArDoCo/Core/blob/10a0a15317dc9aefe070a50e33ecd6d52b19b4cd/cli/src/main/resources/simplelogger.properties#L13 as an example.

@tsaglam
Copy link
Member Author

tsaglam commented Sep 29, 2022

See https://github.com/ArDoCo/Core/blob/10a0a15317dc9aefe070a50e33ecd6d52b19b4cd/cli/src/main/resources/simplelogger.properties#L13 as an example.

I tried that before this fix, did not work here. CoreNLP was still logging.

@dfuchss
Copy link
Member

dfuchss commented Sep 29, 2022

Ah .. we have to adapt our logger. Nevertheless, we should not change log levels programmatically

@tsaglam
Copy link
Member Author

tsaglam commented Sep 29, 2022

Ah .. we have to adapt our logger. Nevertheless, we should not change log levels programmatically

I will make an issue for this. for now we use the hotfix due to time constraints.

@tsaglam tsaglam mentioned this pull request Sep 29, 2022
Closed
@tsaglam tsaglam merged commit 5bea13e into master Sep 29, 2022
@JanWittler JanWittler deleted the suppress-stanford-logger branch September 29, 2022 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
enhancement Issue/PR that involves features, improvements and other changes minor Minor issue/feature/contribution/change
Projects
No open projects
v4.0.0 Release
Status: Done
Milestone
v4.0.0
Development

Successfully merging this pull request may close these issues.
2 participants
@tsaglam @dfuchss