jpos / jPOS

jPOS Project
http://jpos.org
GNU Affero General Public License v3.0
612 stars 461 forks source link

Add support `maxdays` to DailyLogListener #141

Closed ar closed 6 years ago

ar commented 7 years ago

Similar to RotateLogListener, it would be nice to have the ability to automatically erase old log files.

alcarraz commented 7 years ago

Would it be ok to have a max-age property, and deleting files with old timestamps? Difference with RotateLogListener is that e don't have the age (and is not really simple to derive from the name due to the format ability).

Anyway we could do simple patches to DailyLogListener but maybe it would need to be deprecated in favor of something like (Cron|Quartz)RoteateLogListener based on quartz or something similar like cron4j.

ar commented 7 years ago

We have a quartz module in jPOS-EE, works great, but I'd like to keep it there and not add it to jPOS. Perhaps it's easier to deal with a copies and just keep the most recent n copies, like we do with RotateLogListener. For max-age I think we need to rely on the operating system to give us the creation date, if we can get that accurately across OSs, then I'm fine too. We'll have to experiment a bit.

alcarraz commented 7 years ago

Problem with DailyLogListener is that the flexibility in the naming format does not accurately allow to derive neither the creation order nor the files created by the logger only by the names. We could guess the created files given prefix and postfix properties to get the log count. But we could be removing log files intended to be saved by an operator for example by issuing a rename or copy command that maintains the prefix and postfix. I agree this is an improbable scenario and a really bad practice so this doesn't bother me and that problem occurs with max-age too.

But I think we need to rely on the OS modification date to derive which copies to delete.

Either way I would offer the two properties, max-files and max-age.

If you agree with that I'm willing to take the ticket and implement it as soon as I get some time to spare.

El 7 oct. 2017 4:42 PM, "Alejandro Revilla" notifications@github.com escribió:

We have a quartz module in jPOS-EE, works great, but I'd like to keep it there and not add it to jPOS. Perhaps it's easier to deal with a copies and just keep the most recent n copies, like we do with RotateLogListener. For max-age I think we need to rely on the operating system to give us the creation date, if we can get that accurately across OSs, then I'm fine too. We'll have to experiment a bit.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/jpos/jPOS/issues/141#issuecomment-334961046, or mute the thread https://github.com/notifications/unsubscribe-auth/ADmou0rGA9nkmWMSvblqVRslKBJaPkaSks5sp9QRgaJpZM4PxEjM .

ar commented 7 years ago

Sure. Appreciate that. Wonder if we want to go with modification date. On a second thought, I talked about creation date, but modification date is good too, if for some reason an operator has edited the file, it might just mean she wants to keep it for a little longer, so no harm keeping it.

alcarraz commented 7 years ago

I tend to overthink, but would it be worthy to add an option to select the modification or creation date usage?

I don't know if all OSs have record of the two dates. At least it seems linux does not and the Basic File class only provides lastModificationTime, we could use java.nio.file.attribute and use the creationTime but at least in some basic testing I did in my machine it returns the same for the two attributes.

So I don't know if it worthy the extra code to obtain that attributes from the java.nio.file package.

ar commented 7 years ago

I don't think it's required, lastModificationTime is good enough.

ar commented 6 years ago

This has been implemented in PR181 (https://github.com/jpos/jPOS/pull/181). Property is actually called maxage.