Skip to content

Conversation

@vorburger
Copy link
Member

@vorburger vorburger commented Nov 7, 2020

FINERACT-1295

PS: Originally related to (but doesn't solve) FINERACT-1168, but can really be independently reviewed and merged - or not.

@vorburger vorburger force-pushed the FINERACT-1168_stderr branch from c841ebd to ee9010d Compare November 7, 2020 13:26
@ptuomola
Copy link
Contributor

ptuomola commented Nov 8, 2020

Just wondering... do we want to make this the default behaviour? Or just something that is used when running integration tests? I personally think seeing the INFO would be OK as normal behaviour (as that's what people would expect to see from other Spring Boot apps) - and we would just turn it off for Integration Tests. But not sure how easy that is to achieve?

@vorburger
Copy link
Member Author

I thought about this, and while using it actually started liking it better like this, it's suddenly so... "quiet, and clean"! ;-) It also "feels" like it's "lighter" and"starts faster", although that's just "perception", of course. But YMMV. It's a bit like Maven (which verbosely logs everything it's doing) VS Gradle (which spews less logs). This also feels like it's more inline with our https://github.com/apache/fineract#logging-guidelines, notably the point about log levels. So my vote is on just changing this generally, not just for ITs, but I'm interested in how others feel about it.

@vorburger vorburger force-pushed the FINERACT-1168_stderr branch from ee9010d to 8331233 Compare November 12, 2020 19:41
@vorburger
Copy link
Member Author

What do other recently active commiters think re this - pro or con? @vidakovic @avikganguly01 @fynmanoj @percyashu @awasum

@xurror
Copy link
Contributor

xurror commented Nov 14, 2020

I understand this does not resolve https://issues.apache.org/jira/browse/FINERACT-1168 but I think this change only looks good if we can get https://issues.apache.org/jira/browse/FINERACT-764. So, just a thought instead having a foreplay with this logging why not just package it with #1444 or bring it in after.
But overall, I think it's a good fit though.

@avikganguly01
Copy link
Contributor

@vorburger : Logstash scripts in some older implementations will break if we upgrade but that's fine.

@vorburger
Copy link
Member Author

@avikganguly01 do you think it's useful to have INFO logs and has any real world in your logstash? My experience in running Fineract.dev and using Google Error Reporting has been that all I ever look at and seem to care about is WARN and ERROR... (BTW note all the issues linked from FINERACT-932 which I've been able to find! Do you want to contribute fixes for some of them?)

@ptuomola
Copy link
Contributor

ptuomola commented Dec 2, 2020

@vorburger just to be clear, I'm happy either way... haven't tried this to see how much cleaner it is, but at the same time I don't think there's a huge amount of useful information in the INFO logging either.

@github-actions
Copy link

github-actions bot commented Jan 2, 2021

This pull request seems to be stale. Are you still planning to work on it? We will automatically close it in 30 days.

@github-actions github-actions bot added the stale label Jan 2, 2021
@vorburger
Copy link
Member Author

FINERACT-1295

@vorburger
Copy link
Member Author

@vorburger vorburger merged commit 069dae0 into apache:develop Feb 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants