Hi Animesh, Tested all those changes to detail. Those lines were removed due to unexpected behavior that I had not spotted until now.
They were suppose to allow for better fall over between multiple domain controllers, how ever they were causing caching to occur. This meant if a users password was reset in LDAP the old password was still allowing login for a limited time. Please pull the changes forward, Thanks Ian. On 29 Jan 2014 00:07, "Animesh Chaturvedi" <animesh.chaturv...@citrix.com> wrote: > If I look at this commit for example > > > https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;a=commitdiff;h=92b4f66d73562e4211d2d787554ff229dbeb5705 > > It removes the two lines from LdapContextFactory.java > > environment.put("com.sun.jndi.ldap.read.timeout", "500");- > environment.put("com.sun.jndi.ldap.connect.pool", "true"); > > Is that reported by find bug? I don’t know this code so not sure if it is > intentional or not ? > > The point is there may be unintended risks in allowing late changes. > > > > -----Original Message----- > From: Animesh Chaturvedi [mailto:animesh.chaturv...@citrix.com] > Sent: Tuesday, January 28, 2014 3:35 PM > To: dev@cloudstack.apache.org > Subject: RE: Findbugs report on 4.3-forward > > Are you sure all of the ones are needed. A quick look at 20+ commits from > Daan show many formatting changes that may not be necessary and hinder > quick review. > > -----Original Message----- > From: Hugo Trippaers [mailto:trip...@gmail.com] > Sent: Tuesday, January 28, 2014 3:16 PM > To: dev@cloudstack.apache.org > Subject: Re: Findbugs report on 4.3-forward > > > > Sent from my iPhone > > > On 28 jan. 2014, at 23:50, Animesh Chaturvedi < > animesh.chaturv...@citrix.com> wrote: > > > > > > -----Original Message----- > > From: Hugo Trippaers [mailto:trip...@gmail.com] > > Sent: Tuesday, January 28, 2014 2:37 PM > > To: dev@cloudstack.apache.org > > Cc: dev@cloudstack.apache.org > > Subject: Re: Findbugs report on 4.3-forward > > > > Hey Animesh, > > > > Not in agreement here. These are squashed bugs and we want as less bugs > in the release as possible. > > [Animesh] I understand but once we enter RC phase we only limit > important fixes. I have pulled in 2 commits from yours and 1 from Daan. > > We limited our fixes to only the important issues that we found. The other > 6000 issues between coverity and findbugs are still being triaged and will > probably not make it into this release. > > > > > This is why we test any RC before we release it. > > [Animesh] Of course but timing is a bit off, if this was done a month > back it would have been fine. > > I say include all the big fixes we have in the release. If that means > more testing before we cut the RC then that is what it is. I can't > rightfully vote for a release with known issues with existing fixes. > > [Animesh] Any release will have known issues, if we have fixes but can't > be sure on regression impact then we have to make a choice. > > Agreed, but we just don't agree on what that choice should be yet ;-) > > > Quality over release schedule would be my vote then. > > [Animesh] But why so late? Why was this activity not planned early on? I > have been reminding community to call out issues early on since like > mid-December. > > On November 4 I sent the mail to the dev list that static code analysis > (coverity) found 6000+ issues that needed to be triaged. I worked on quite > a few with my colleagues, but it's a big task for just the four of us. > Findbugs just helped us to quickly identify the real scary issues among > them. > So I agree that the timing is less than ideal, but we should do our utmost > best to ship the highest quality piece of software we can. > > > > > Cheers, > > Hugo > > > > Sent from my iPhone > > > >> On 28 jan. 2014, at 18:48, Animesh Chaturvedi < > animesh.chaturv...@citrix.com> wrote: > >> > >> Folks these issues reported by find-bugs have existed for some time. I > am not confident in picking them up now for 4.3 as it may break code that > assumed old way of working. We can take them up for 4.3 maintenance > release. I wish we had done this exercise and not waited until now. > >> > >> I will pick Hugo's commit for which he called -1. > >> > >> Thanks > >> Animesh > >> > >> -----Original Message----- > >> From: Trippie [mailto:trip...@gmail.com] On Behalf Of Hugo Trippaers > >> Sent: Tuesday, January 28, 2014 1:29 AM > >> To: dev > >> Subject: Re: Findbugs report on 4.3-forward > >> > >> Hey Animesh, > >> > >> I agree with Daan here. We focussed on the bugs with a findbugs > annotation of scariest. I think that would warrant them to be included in > the 4.3 release, so please cherry-pick them all. > >> > >> > >> Cheers, > >> > >> Hugo > >> > >>> On 28 jan. 2014, at 09:32, Daan Hoogland <daan.hoogl...@gmail.com> > wrote: > >>> > >>> Animesh, > >>> > >>> I took up a lot of messages from findbugs in the server package over > >>> the weekend. Not that I will attach my soul to the shipping of my > >>> fixes but some of them are == vs eq and some are really nasty > >>> nullpointer issues (a chack after first use is very common). You can > >>> cherry-pick them or not. I don't think you should leave any of them > >>> behind. Even with David being right we should go against him at > >>> every convenient time, running the risk of being called his wife. > >>> > >>>> On Tue, Jan 28, 2014 at 6:25 AM, Ian Duffy <i...@ianduffy.ie> wrote: > >>>> Hi Animesh, > >>>> > >>>> Can you cherry-pick the below commit from from 4.3-forward to 4.3 > branch? > >>>> > >>>> Fix findbug issues within LDAP authenticator commit > >>>> 92b4f66d73562e4211d2d787554ff229dbeb5705 > >>>> > >>>> Thanks, > >>>> Ian > >>>> > >>>> On 28 January 2014 03:48, Animesh Chaturvedi > >>>> <animesh.chaturv...@citrix.com>wrote: > >>>> > >>>>> Hugo I was reviewing your commits to 4.3-forward and looked at > >>>>> your commits > >>>>> > >>>>> > >>>>> https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;a=commit; > >>>>> h = f18c5a1910b6370585a1d61638b8310c3ecba5ef > >>>>> > >>>>> https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;a=commit; > >>>>> h = 60ac12780bfa1604902a89d5dc7937a8b9334e0d > >>>>> I think you want the last one which has fixes for NetUtils and > >>>>> XenServerStorageMotionStrategy for which you had put -1 in first > >>>>> RC but the commit includes more files. Can you make limited > >>>>> changes directly to 4.3? I want to build another RC later tonight > >>>>> > >>>>> Animesh > >>>>> > >>>>> > >>>>> -----Original Message----- > >>>>> From: Animesh Chaturvedi [mailto:animesh.chaturv...@citrix.com] > >>>>> Sent: Monday, January 27, 2014 1:30 PM > >>>>> To: dev@cloudstack.apache.org > >>>>> Subject: RE: Findbugs report on 4.3-forward > >>>>> > >>>>> Agreed > >>>>> > >>>>> We need to fix the most important ones for 4.3. There may be > >>>>> assumptions in the code which we may not know and may get broken > >>>>> if these issues are fixed late. I will pull in the one Hugo casted > >>>>> his > >>>>> -1 for the first vote, any others? > >>>>> > >>>>> Animesh > >>>>> > >>>>> -----Original Message----- > >>>>> From: David Nalley [mailto:da...@gnsa.us] > >>>>> Sent: Monday, January 27, 2014 11:46 AM > >>>>> To: dev@cloudstack.apache.org > >>>>> Subject: Re: Findbugs report on 4.3-forward > >>>>> > >>>>> So just curious if I am the only one concerned about a ton of > >>>>> fixes going in at the last minute. If the fixes are for serious > >>>>> bugs and we have consensus around their severity being high enough, > indeed lets fix things. > >>>>> My concern is that much of the QA we do is manual; and while we > >>>>> are getting better; fixing tons of things at the last minute may > >>>>> have unintended consequences that we don't know about and won't > easily find. > >>>>> > >>>>> I yearn for the day when our automated testing is broad enough > >>>>> that we can do fixes right up to the wire and know that things > >>>>> still work, I am just not sure that I have confidence that we are > there yet. > >>>>> Thoughts? I am being paranoid? > >>>>> > >>>>> --David > >>>>> > >>>>> On Mon, Jan 27, 2014 at 3:11 AM, Daan Hoogland > >>>>> <daan.hoogl...@gmail.com> > >>>>> wrote: > >>>>>> Animesh, I commented the once i made yesterday with findbugs: > >>>>>> > >>>>>> I allready send a few and will get you a list of the rest later > today. > >>>>>> > >>>>>> regards, > >>>>>> > >>>>>> On Mon, Jan 27, 2014 at 3:48 AM, Animesh Chaturvedi > >>>>>> <animesh.chaturv...@citrix.com> wrote: > >>>>>>> Good job fellas. I see a number of commits 20+ into 4.3-forward > branch. > >>>>> Are their specific commits you want me to pick up out of these? > >>>>>>> > >>>>>>> Animesh > >>>>>>> > >>>>>>> -----Original Message----- > >>>>>>> From: Daan Hoogland [mailto:daan.hoogl...@gmail.com] > >>>>>>> Sent: Sunday, January 26, 2014 2:41 AM > >>>>>>> To: dev > >>>>>>> Subject: Re: Findbugs report on 4.3-forward > >>>>>>> > >>>>>>> I didn't get very far last night and will be looking at the > >>>>>>> server > >>>>> package again this afternoon. > >>>>>>> > >>>>>>> bon appétit, > >>>>>>> > >>>>>>>> On Sun, Jan 26, 2014 at 1:36 AM, Ian Duffy <i...@ianduffy.ie> > wrote: > >>>>>>>> Hi, > >>>>>>>> > >>>>>>>> Fixed the issues highlighted in the ldap user authentication > package. > >>>>>>>> > >>>>>>>> Have pushed to 4.3-forward. > >>>>>>>> > >>>>>>>> Thanks, > >>>>>>>> Ian > >>>>>>>> > >>>>>>>> > >>>>>>>> On 25 January 2014 22:26, Daan Hoogland > >>>>>>>> <daan.hoogl...@gmail.com> > >>>>> wrote: > >>>>>>>> > >>>>>>>>>> or reply to this mail with the filename you are working on > >>>>>>>>> I'll be looking at the server package as it seems to contain > >>>>>>>>> the most issues. > >>>>>>>>> > >>>>>>>>> On Sat, Jan 25, 2014 at 4:00 PM, Hugo Trippaers > >>>>>>>>> <h...@trippaers.nl> > >>>>> wrote: > >>>>>>>>>> I've also added a job to master with the Findbugs report and > >>>>>>>>>> the > >>>>>>>>> cobertura code coverage report. > >>>>>>>>>> > >>>>>>>>>> Good stuff, we have a 12% coverage of our classes with unit > tests. > >>>>>>>>>> Huge > >>>>>>>>> improvement over the last release where we had 4% iirc. We > >>>>>>>>> have > >>>>>>>>> 306 reports from Findbugs, of which the majority are > >>>>>>>>> internationalization > >>>>> issues. > >>>>>>>>> (String.getBytes without charset mostly). On the coverity site > >>>>>>>>> we have > >>>>>>>>> 6000+ issues still open, but at least that number is > >>>>>>>>> 6000+ relatively stable, we > >>>>>>>>> fix as much issues as we introduce and it's untuned so we can > >>>>>>>>> assume a large number of false positives there. > >>>>>>>>>> > >>>>>>>>>> I think that on average the automated tools tell us that code > >>>>>>>>>> quality is > >>>>>>>>> improving, which a good thing. Combined with the functional > >>>>>>>>> testing and the simulator build we can prove that we are doing > >>>>>>>>> quite well on the code quality angle. > >>>>>>>>>> > >>>>>>>>>> http://jenkins.buildacloud.org/job/build-master-slowbuild/ > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> Cheers, > >>>>>>>>>> > >>>>>>>>>> Hugo > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> On 25 jan. 2014, at 14:13, Daan Hoogland > >>>>>>>>>> <daan.hoogl...@gmail.com> > >>>>>>>>> wrote: > >>>>>>>>>> > >>>>>>>>>>> H Hugo, > >>>>>>>>>>> > >>>>>>>>>>> I'll spend some time on it tonight. Do you have a work load > >>>>>>>>>>> distribution scheme or is it random access? > >>>>>>>>>>> ;) > >>>>>>>>>>> > >>>>>>>>>>> regards > >>>>>>>>>>> > >>>>>>>>>>> On Sat, Jan 25, 2014 at 12:39 PM, Hugo Trippaers > >>>>>>>>>>> <h...@trippaers.nl> > >>>>>>>>> wrote: > >>>>>>>>>>>> Hey all, > >>>>>>>>>>>> > >>>>>>>>>>>> I've made Jenkins run the findbugs analysis on 4.3-forward. > >>>>>>>>>>>> Is there > >>>>>>>>> somebody who is willing to help triage the findings? Maybe > >>>>>>>>> there is some stuff that we need to fix? > >>>>>>>>>>>> > >>>>>>>>>>>> the url is > >>>>>>>>> http://jenkins.buildacloud.org/job/cloudstack-4.3-forward-mave > >>>>>>>>> n > >>>>>>>>> - > >>>>>>>>> bui > >>>>>>>>> ld > >>>>>>>>> /3/findbugsResult/ > >>>>>>>>>>>> > >>>>>>>>>>>> Cheers, > >>>>>>>>>>>> > >>>>>>>>>>>> Hugo > >> >
