Also, I'm with the proposal by Clebert of just writing the tests and making sure they are good and then continuing the release. If we went with that approach then we could just do the reload4j upgrade for 5.16.5.
On Fri, Feb 4, 2022 at 7:38 AM Robbie Gemmell <robbie.gemm...@gmail.com> wrote: > I similarly question doing 5.15.x given 5.15.15 was announced as the > last 5.15.x release and for folks to upgrade. It has been just over 9 > months since then, with the 5.16.3 release nearly 6 months ago now > containing various updates and the 5.16.4-in-progress containing more > where it seems like if those weren't important enough to punt back > already, its not clear this should be now either. > > Seems more overdue to just remove it from the download page and > consider it done as the page/announcement stated. > > On Fri, 4 Feb 2022 at 11:48, Christopher Shannon > <christopher.l.shan...@gmail.com> wrote: > > > > I don't think it's needed for 5.15.x as I don't believe there are plans > to > > do anymore of those releases. However for 5.16.x it makes a lot of sense > > since we are only going with Log4j2 on 5.17.x and newer. > > > > So definitely +1 for me to replace it in 5.16.x. > > > > On Fri, Feb 4, 2022 at 6:20 AM Jonathan Gallimore < > > jonathan.gallim...@gmail.com> wrote: > > > > > I had wondered the same thing. I'd be +1 on it. > > > > > > Jon > > > > > > On Fri, Feb 4, 2022 at 11:17 AM Colm O hEigeartaigh < > cohei...@apache.org> > > > wrote: > > > > > > > Hey JB/all, > > > > > > > > I'm wondering if we can also take the opportunity to consider > > > > switching from Apache Log4J 1.x to Reload4J for AMQ 5.15.x/5.16.x. > > > > https://reload4j.qos.ch/ is a fork of Log4J 1.x with the CVE fixes > > > > applied. Other Apache projects such as Apache Avro have switched over > > > > to use it, similarly pax logging > > > > ( > > > > > > > > https://github.com/ops4j/org.ops4j.pax.logging/commit/a21b1b41fb107164522e587643e8daff2724ce86 > > > > ). > > > > > > > > There will be many users of AMQ who will not want or be able to > > > > migrate to AMQ 5.17 and hence Log4J 2. It's difficult to convey to > > > > users that the CVEs don't apply to the default configuration, as many > > > > users just see critical CVEs via their own scanning tools and insist > > > > on upgrades. In addition, Log4J 1.x is not maintained since 2015 and > > > > if a new CVE were to come out that affects the default configuration, > > > > it would be problematic. > > > > > > > > If you approve I can create a PR. > > > > > > > > Colm. > > > > > > > > On Fri, Feb 4, 2022 at 7:51 AM Jean-Baptiste Onofré <j...@nanthrax.net > > > > > > wrote: > > > > > > > > > > Hi Clebert, > > > > > > > > > > Good idea ! > > > > > > > > > > If Chris agree with this plan, I'm fine with it. > > > > > > > > > > Let me work on the tests then (I will create PRs). > > > > > > > > > > Regards > > > > > JB > > > > > > > > > > On 04/02/2022 00:55, Clebert Suconic wrote: > > > > > > I would say.. write the tests... if they pass no need to cancel > the > > > > > > release as they will not affect runtime. only cancel the release > if > > > > > > you actually find an issue. > > > > > > > > > > > > On Thu, Feb 3, 2022 at 8:15 AM Jean-Baptiste Onofré < > j...@nanthrax.net > > > > > > > > wrote: > > > > > >> > > > > > >> Hi Chris, > > > > > >> > > > > > >> OK, fair enough for me. I will cancel the vote and work with > Matt to > > > > add > > > > > >> the tests. > > > > > >> > > > > > >> Thanks, > > > > > >> Regards > > > > > >> JB > > > > > >> > > > > > >> On 03/02/2022 13:57, Christopher Shannon wrote: > > > > > >>> JB, > > > > > >>> > > > > > >>> I would rather just add tests first and re-run the vote as it > > > > sounded like > > > > > >>> it wouldn't take too long to add a few tests. I wanted to > verify > > > > some of > > > > > >>> those fixes work and make sense and it would be a lot easier > not > > > > having to > > > > > >>> try and figure out how to set up a manual test that will most > > > likely > > > > be > > > > > >>> inconsistent from what others used depending on the config > settings > > > > and > > > > > >>> testing scenarios. Furthermore, it's very common to think an > issue > > > > is fixed > > > > > >>> from manual testing and then run through unit tests and then > find > > > an > > > > issue > > > > > >>> (missing edge case, etc) > > > > > >>> > > > > > >>> Matt, for AMQ-8397 you are right that it would be tricky to > test > > > the > > > > > >>> behavior itself but I don't think that's necessary. Since the > fix > > > is > > > > really > > > > > >>> just about adding a new flag to disable sending to the DLQ I > would > > > > say > > > > > >>> instead of testing the full behavior you could at least just > write > > > a > > > > quick > > > > > >>> test that would verify the default is what you want (off by > > > default) > > > > and > > > > > >>> that changing the policy actually properly sets it on the > > > > destination as > > > > > >>> intended. > > > > > >>> > > > > > >>> On Thu, Feb 3, 2022 at 3:44 AM Jean-Baptiste Onofré < > > > j...@nanthrax.net> > > > > wrote: > > > > > >>> > > > > > >>>> Hi, > > > > > >>>> > > > > > >>>> As some features have been tested manually (you did a pass, I > did > > > a > > > > > >>>> quick pass on some as well), it's good enough for the release. > > > > > >>>> Agree with Chris that we should add test to "secure" the code > > > > > >>>> coverage/regression, but it should be done. > > > > > >>>> Either way is fine for me: if we consider it's blocker, I can > > > > cancel the > > > > > >>>> current vote, include new tests and cut a new release. > > > > > >>>> Else, I would say, let's move forward with the current vote > and > > > > include > > > > > >>>> tests asap (decoupled from this vote/release). > > > > > >>>> > > > > > >>>> Regards > > > > > >>>> JB > > > > > >>>> > > > > > >>>> On 02/02/2022 22:21, Matt Pavlovich wrote: > > > > > >>>>> Note— I did test these changes manually. > > > > > >>>>> > > > > > >>>>>> On Feb 2, 2022, at 2:55 PM, Matt Pavlovich < > mattr...@gmail.com> > > > > wrote: > > > > > >>>>>> > > > > > >>>>>> Hey Christopher- > > > > > >>>>>> > > > > > >>>>>> I see your point on the wording of AMQ-8443. It is an edge > case > > > > with > > > > > >>>> ConnectionControl, not failover in general. I’ll clear up the > > > > description. > > > > > >>>> I’m happy to add tests for a couple of them; however, > AMQ-8397 is > > > > tricky. > > > > > >>>>>> > > > > > >>>>>> What do you suggest for a reliable test scenario for > AMQ-8397? > > > > > >>>>>> > > > > > >>>>>> AMQ-8053 - This is a very minor change, I’ll add a test > > > > > >>>>>> AMQ-8443 - Edge case, I’ll add a test > > > > > >>>>>> AMQ-8397 - Note— this is ‘disabled by default in 5.16.x’. > This > > > is > > > > a > > > > > >>>> reporting change on an edge case from being “move message to > DLQ” > > > > to log > > > > > >>>> message and increment a counter. This is really difficult to > > > > consistently > > > > > >>>> create a unit test to produce the scenario on varying > hardware. > > > > This is a > > > > > >>>> heavy contention scenario and I was not able to find any tests > > > that > > > > produce > > > > > >>>> the current behavior. I’d be happy to adapt verification if > you > > > can > > > > provide > > > > > >>>> a stable base scenario (or point to an existing unit test > that I > > > > may have > > > > > >>>> missed). > > > > > >>>>>> > > > > > >>>>>> Thanks, > > > > > >>>>>> Matt Pavlovich > > > > > >>>>>> > > > > > >>>>>>> On Feb 2, 2022, at 12:56 PM, Christopher Shannon < > > > > > >>>> christopher.l.shan...@gmail.com <mailto: > > > > christopher.l.shan...@gmail.com>> > > > > > >>>> wrote: > > > > > >>>>>>> > > > > > >>>>>>> Unfortunately I am -1. I just checked the release and > several > > > > Jiras > > > > > >>>> don't > > > > > >>>>>>> have any unit tests for verification. > > > > > >>>>>>> > > > > > >>>>>>> https://issues.apache.org/jira/browse/AMQ-8053 < > > > > > >>>> https://issues.apache.org/jira/browse/AMQ-8053> > > > > > >>>>>>> https://issues.apache.org/jira/browse/AMQ-8372 > > > > > >>>>>>> https://issues.apache.org/jira/browse/AMQ-8397 > > > > > >>>>>>> https://issues.apache.org/jira/browse/AMQ-8443 > > > > > >>>>>>> > > > > > >>>>>>> There might be more, I quit checking after the first > several. > > > > All of > > > > > >>>> these > > > > > >>>>>>> fixes and improvements need tests. It's important to add > some > > > > tests to > > > > > >>>>>>> verify the actual bug or issue and to make sure it doesn't > get > > > > broken > > > > > >>>> in > > > > > >>>>>>> the future. I am not comfortable releasing this without > having > > > > tests > > > > > >>>> for > > > > > >>>>>>> these issues. The same of course goes into anything with > > > 5.17.0. > > > > > >>>> Obviously > > > > > >>>>>>> we don't need tests for trivial stuff (dependency upgrades, > > > null > > > > > >>>> pointer > > > > > >>>>>>> checks, etc) but other stuff should be tested. > > > > > >>>>>>> > > > > > >>>>>>> The biggest issue I have is AMQ-8443 which claims this is > a fix > > > > for the > > > > > >>>>>>> "failover transport reconnect not working" but no > demonstration > > > > of the > > > > > >>>>>>> issue or how the fix works in a unit test. > > > > > >>>>>>> > > > > > >>>>>>> On Wed, Feb 2, 2022 at 3:29 AM Francois Papon < > > > > > >>>> francois.pa...@openobject.fr> > > > > > >>>>>>> wrote: > > > > > >>>>>>> > > > > > >>>>>>>> +1 (non-binding) > > > > > >>>>>>>> > > > > > >>>>>>>> Thanks! > > > > > >>>>>>>> > > > > > >>>>>>>> regards, > > > > > >>>>>>>> > > > > > >>>>>>>> François > > > > > >>>>>>>> > > > > > >>>>>>>> On 01/02/2022 21:41, Jean-Baptiste Onofre wrote: > > > > > >>>>>>>>> Hi everyone, > > > > > >>>>>>>>> > > > > > >>>>>>>>> I submit Apache ActiveMQ 5.16.4 release to your vote. > > > > > >>>>>>>>> > > > > > >>>>>>>>> This release includes important fixes and updates on the > > > 5.16.x > > > > > >>>> series, > > > > > >>>>>>>> especially: > > > > > >>>>>>>>> - fix couple of warnings/issues on JDK16+ > > > > > >>>>>>>>> - fix stack trace display on transports > > > > > >>>>>>>>> - better secure on WebConsole > > > > > >>>>>>>>> - several dependencies updates > > > > > >>>>>>>>> - and much more! > > > > > >>>>>>>>> > > > > > >>>>>>>>> Please take a look on the Release Notes for details: > > > > > >>>>>>>>> > > > > > >>>>>>>> > > > > > >>>> > > > > > > > > https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210&version=12350483 > > > > > >>>>>>>>> > > > > > >>>>>>>>> Maven staging repository is: > > > > > >>>>>>>>> > > > > > >>>>>>>> > > > > > >>>> > > > > > > > > https://repository.apache.org/content/repositories/orgapacheactivemq-1244/ > > > > > >>>>>>>>> > > > > > >>>>>>>>> Dist staging repository is: > > > > > >>>>>>>>> > > > > https://dist.apache.org/repos/dist/dev/activemq/activemq/5.16.4/ > > > > > >>>>>>>>> > > > > > >>>>>>>>> Git tag: > > > > > >>>>>>>>> activemq-5.16.4 > > > > > >>>>>>>>> > > > > > >>>>>>>>> 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 > > > > > >>>>>>>> > > > > > >>>>>> > > > > > >>>>> > > > > > >>>>> > > > > > >>>> > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > >
