You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by GitBox <gi...@apache.org> on 2020/05/03 16:03:02 UTC
[GitHub] [fineract] vorburger opened a new pull request #813: add (Quartz's) LoggingTriggerHistoryPlugin (re. FINERACT-922)
vorburger opened a new pull request #813:
URL: https://github.com/apache/fineract/pull/813
To have more visibility into scheduler jobs.
FINERACT-922
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vorburger commented on pull request #813: add (Quartz's) LoggingTriggerHistoryPlugin (re. FINERACT-922)
Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #813:
URL: https://github.com/apache/fineract/pull/813#issuecomment-626255312
/rebase
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vorburger commented on pull request #813: add (Quartz's) LoggingTriggerHistoryPlugin (re. FINERACT-922)
Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #813:
URL: https://github.com/apache/fineract/pull/813#issuecomment-633115608
@awasum @ptuomola review and merge this?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] awasum commented on pull request #813: add (Quartz's) LoggingTriggerHistoryPlugin (re. FINERACT-922)
Posted by GitBox <gi...@apache.org>.
awasum commented on pull request #813:
URL: https://github.com/apache/fineract/pull/813#issuecomment-633282901
> Looking at the source code, this plugin logs at loglevel INFO. Given the logging guidelines, do we want to therefore enable "permanently" i.e. including non-test configurations?
>
> An option could be to add this to a property file so you can then comment the property in / out when testing - but without needing to recompile:
>
> org.quartz.plugin.triggHistory.class =
> org.quartz.plugins.history.LoggingTriggerHistoryPlugin
Is this something you can handle in a follow up PR? @ptuomola Or Is this critical to the usefulness of this 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vorburger commented on pull request #813: add (Quartz's) LoggingTriggerHistoryPlugin (re. FINERACT-922)
Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #813:
URL: https://github.com/apache/fineract/pull/813#issuecomment-625807576
Please don't merge this one just yet; I want to do some more local tests to compare this VS #812.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] awasum commented on pull request #813: add (Quartz's) LoggingTriggerHistoryPlugin (re. FINERACT-922)
Posted by GitBox <gi...@apache.org>.
awasum commented on pull request #813:
URL: https://github.com/apache/fineract/pull/813#issuecomment-633281184
/rebase
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] awasum commented on pull request #813: add (Quartz's) LoggingTriggerHistoryPlugin (re. FINERACT-922)
Posted by GitBox <gi...@apache.org>.
awasum commented on pull request #813:
URL: https://github.com/apache/fineract/pull/813#issuecomment-633434442
I made a mistake and rebased this yesterday. It was not necessary.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vorburger commented on pull request #813: add (Quartz's) LoggingTriggerHistoryPlugin (re. FINERACT-922)
Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #813:
URL: https://github.com/apache/fineract/pull/813#issuecomment-632696643
> Please don't merge this one just yet; I want to do some more local tests to compare this VS #812.
Let's actually merge this... together with, not instead of #812 - having both shouldn't do any harm - more logging for eyes is probably a very Good Thing in the case of the scheduler, with all the problems we're having 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola commented on pull request #813: add (Quartz's) LoggingTriggerHistoryPlugin (re. FINERACT-922)
Posted by GitBox <gi...@apache.org>.
ptuomola commented on pull request #813:
URL: https://github.com/apache/fineract/pull/813#issuecomment-634209934
> Is this something you can handle in a follow up PR? @ptuomola Or Is this critical to the usefulness of this PR?
Well... this is a one liner PR to introduce logging for triggers. I'm suggesting another oneliner change that should achieve exactly the same thing but make it easy to turn on/off, so allowing this change to comply with our logging policy.
So of course we can merge this PR and then someone else can raise another PR to remove the line added here and add another line instead. Or we can just change this PR to use the other line straightaway :-). Either is naturally OK with me, so will let @vorburger decide how he'd like to handle this...
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vorburger commented on pull request #813: add (Quartz's) LoggingTriggerHistoryPlugin (re. FINERACT-922)
Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #813:
URL: https://github.com/apache/fineract/pull/813#issuecomment-632696065
/rebase
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vorburger commented on pull request #813: add (Quartz's) LoggingTriggerHistoryPlugin (re. FINERACT-922)
Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #813:
URL: https://github.com/apache/fineract/pull/813#issuecomment-634214075
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vorburger commented on pull request #813: add (Quartz's) LoggingTriggerHistoryPlugin (re. FINERACT-922)
Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #813:
URL: https://github.com/apache/fineract/pull/813#issuecomment-636692368
> add this to a property file so you can then comment the property in / out when testing - but without needing to recompile: `org.quartz.plugin.triggHistory.class = org.quartz.plugins.history.LoggingTriggerHistoryPlugin`
@ptuomola yeah, makes sense, as per http://www.quartz-scheduler.org/documentation/quartz-2.3.0/configuration/ConfigPlugins.html. But actually, looking at this against after a bit of time, and re-comparing it with my other #812, I see that this probably won't really add any more additional useful information anyway, and the logging levels in (my) `SchedulerTriggerListener` are better.
>> Please don't merge this one just yet; I want to do some more local tests to compare this VS #812.
> Let's actually merge this... together with, not instead of #812 - having both shouldn't do any harm -
I'll therefore "abandon" (close without merge) this proposal, 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vorburger removed a comment on pull request #813: add (Quartz's) LoggingTriggerHistoryPlugin (re. FINERACT-922)
Posted by GitBox <gi...@apache.org>.
vorburger removed a comment on pull request #813:
URL: https://github.com/apache/fineract/pull/813#issuecomment-634246590
Wait a sec, so iText v5 is AGPL licensed?! Then we can't use it in Fineract, see https://www.apache.org/legal/resolved.html#category-x ...
Reading https://en.m.wikipedia.org/wiki/IText and seeing on https://search.maven.org/search?q=iText that v4.2.2 is ancient from 2015.
I suggest that we use this as the opportunity to switch to https://github.com/LibrePDF/OpenPDF instead. That's dual LGPL (NOK) and BSD (fine).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola commented on pull request #813: add (Quartz's) LoggingTriggerHistoryPlugin (re. FINERACT-922)
Posted by GitBox <gi...@apache.org>.
ptuomola commented on pull request #813:
URL: https://github.com/apache/fineract/pull/813#issuecomment-633278341
Looking at the source code, this plugin logs at loglevel INFO. Given the logging guidelines, do we want to therefore enable "permanently" i.e. including non-test configurations?
An option could be to add this to a property file so you can then comment the property in / out when testing - but without needing to recompile:
org.quartz.plugin.triggHistory.class = \
org.quartz.plugins.history.LoggingTriggerHistoryPlugin
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vorburger closed pull request #813: add (Quartz's) LoggingTriggerHistoryPlugin (re. FINERACT-922)
Posted by GitBox <gi...@apache.org>.
vorburger closed pull request #813:
URL: https://github.com/apache/fineract/pull/813
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org