Furthermore the regressions in STOMP can be seen here in Jenkins: https://builds.apache.org/view/A/view/ActiveMQ/job/ActiveMQ-Java8/lastCompletedBuild/testReport/
Also looking at another fix that was just merged a couple days ago: I'm not convinced that this is correct: https://issues.apache.org/jira/projects/AMQ/issues/AMQ-7314 There's no test here to demonstrate the fix and it seems like it might be wrong to mark a sequenceId less than the lastDeliveredSequenceId to be the lastDeliveredRef. A change like this needs to demonstrate the issue and fix works My recommendation when doing future releases is to freeze the release earlier in the process and to not try and merge in a ton of stuff in the last couple days before a release. Last minute changes can be pushed to the next release unless critical/blockers. Merging at the last second before a release doesn't give much time for others to review the changes and verify they make sense or to even give CI a chance to run and be verified as shown here. On Mon, Mar 9, 2020 at 8:49 AM Christopher Shannon < [email protected]> wrote: > -1, there are multiple STOMP regressions here > > 1) In some of my tests that verify error handling works properly, I'm > getting a NPE here that causes my build to fail: > https://github.com/apache/activemq/blob/activemq-5.15.12/activemq-stomp/src/main/java/org/apache/activemq/transport/stomp/ProtocolConverter.java#L261 > > The root cause is there's no guarantee there will be a linked exception > and in this case the linked exception is null leading to an NPE > > 2) It makes no sense to swallow the exception here and not propagate it > up: > https://github.com/apache/activemq/blob/3c302dce33500cd8e36f626af333ce208ebc44a0/activemq-stomp/src/main/java/org/apache/activemq/transport/stomp/StompNIOSSLTransport.java#L67 > > Every other protocol throws the IOException and this no longer does. > Furthermore, it makes no sense to treat STOMP different than other > protocols. I don't see why we need to log at all when the IOException > should propagate up and then be logged elsewhere. If it's not being logged > properly then something else up the chain (TcpTransport or whatever) should > probably be catching the exception and logging. > > On Mon, Mar 9, 2020 at 8:33 AM Daniel Kulp <[email protected]> wrote: > >> +1 >> >> Dan >> >> >> > On Mar 6, 2020, at 2:15 AM, Jean-Baptiste Onofre <[email protected]> >> wrote: >> > >> > Hi everyone, >> > >> > I'm submitting ActiveMQ 5.15.12 release to your vote. >> > >> > This release includes dependency updates (especially for CVE/Security >> > fixes), important improvements on JDBC persistence adapter, other >> improvements and fixes. >> > >> > Please take a look on the Release Notes for details: >> > >> > >> https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210&version=12346500 >> < >> https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210&version=12346500 >> > >> > >> > The Maven staging repository is: >> > >> https://repository.apache.org/content/repositories/orgapacheactivemq-1204 >> > >> > The dist staging repository is: >> > https://dist.apache.org/repos/dist/dev/activemq/activemq/5.15.12/ < >> https://dist.apache.org/repos/dist/dev/activemq/activemq/5.15.12/> >> > >> > Git tag: >> > activemq-5.15.12 >> > >> > Website PR: >> > https://github.com/apache/activemq-website/pull/28 < >> https://github.com/apache/activemq-website/pull/28> >> > >> > Please vote to approve this release: >> > >> > [ ] +1 Approve the release >> > [ ] -1 Don't approve the release (please provide specific comments) >> > >> > This vote will be open for at least 72 hours. >> > >> > Thanks ! >> > Regards >> > JB >> >> -- >> Daniel Kulp >> [email protected] <mailto:[email protected]> - http://dankulp.com/blog < >> http://dankulp.com/blog> >> Talend Community Coder - http://talend.com <http://coders.talend.com/> >> >
