gemmellr commented on PR #5303: URL: https://github.com/apache/activemq-artemis/pull/5303#issuecomment-2435720995
I don't think we should be suggesting the lowest/first available non-retired ID when someone reuses a current/retired ID. Those are more likely to be IDs that have been used before and removed, but dont yet have a retiredID value set to prevent their reuse. I also don't think its worth adding 2 dependencies (both seemingly long-inactive) to the build to help do that. We aren't short of new IDs in most cases, so I think recommending the next _highest_ non-retired value makes more sense. I put together a change that drops the new deps and just takes the highest active ID found for the bundle, and keeps incrementing it until one is found that isnt retired, which is then also checked against the regex to verify not exceeding the limit. I also integrated the new check into the existing verification that was verifying ID reuse already. I added some checking that the retiredIDs values match the regex, to help catch mistakes when defining them, and enforced the values given are sorted, both for readability reasons later and impl reasons (and fixed the very few that were not already...using the sorted-for-you version included in the exception message). I also added the class to the start of the exception messages above to make it clearer where the problem being reported is. (I saw you had started printing the class to stdout all the time, I wasn't sure if that was deliberate or just a leftover not-yet-made-debug). https://github.com/gemmellr/activemq-artemis/tree/jbertram-ARTEMIS-5110_rebased has your changes rebased, and another commit (https://github.com/gemmellr/activemq-artemis/commit/87385e76b9d24dd7f8ab829a4a57b486de6f6586 currently) with difference around the above. (May still be bugs, I am pushing it early for comment hehe) What do you think ? -- 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. To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org For additional commands, e-mail: gitbox-h...@activemq.apache.org For further information, visit: https://activemq.apache.org/contact
