search

jpos / jPOS

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

Let the max depth for daily log listener deletion be configurable. #583

Open alcarraz opened 7 months ago

alcarraz commented 7 months ago

Fixes #580 About the rotation of q2 logs

alcarraz commented 7 months ago

It seems I will have to change tests to work on windows, marked as drat until then.

alcarraz commented 7 months ago

Sorry for all the noise regarding this PR, wasn't understanding the real problem behind the temporary directory cleanup on windows, the last failure (java 11, windows-latest) doesn't seem to be related to this PR, but I don't have the permissions to rerun the workflow.

ar commented 7 months ago

I have mixed feelings about this PR. Please see this unanswered comment:

https://github.com/jpos/jPOS/issues/580#issuecomment-1889962931

alcarraz commented 7 months ago

I have mixed feelings about this PR. Please see this unanswered comment:

#580 (comment)

Yes, I get your concerns, I did this based on the part of your comment that states:

Setting DEF_MAXDEPTH to 1 as the default limit for recursive deletion operations provides a safer approach, but I agree with you that this should be configurable for situations where you know what you're doing.

How would you feel about renaming the constant (it's not a default if it is cannot be overridden) or add a getter for max depth that can be overridden by a subclass? This way, it's less likely the user doesn't know what he is doing.

ar commented 7 months ago

I'd like an answer to my question in the issue, if the problem was the final / then there's nothing to fix. I can't imagine a situation where we want to traverse a deep directory hierarchy to find logs to delete.