Please ignore my previous-mail, that was sent accidentally. Usage of AuditLogDirector in VdsBrokerObjectsBuilder is an illustration for a sub-optimal design:
1. The methods it is used in them are checkTimeDrift and reportInvalidInterfacesForNetwork. Both are: 1. static 2. should not be part of the class they are in 3. implement a piece of business logic that should not be in vdsbroker, but in bll 2. vdsbroker module should do the "transformation" and that only. Any other logic is a business one and should be in bll. Best regards, ____________________ Yevgeny Zaspitsky Senior Software Engineer Red Hat Israel 34 Jerusalem Road, Ra'anana, Israel 43501 Tel: +972 9 7692098 Mobile: +972 52 6323656 Email: yzasp...@redhat.com IRC: yzaspits ----- Original Message ----- > From: "Yevgeny Zaspitsky" <yzasp...@redhat.com> > To: "Yair Zaslavsky" <yzasl...@redhat.com> > Cc: devel@ovirt.org > Sent: Monday, October 6, 2014 9:19:31 PM > Subject: Re: [ovirt-devel] [ENGINE] splitting AuditLogDirector to its own > module > Usage of AuditLogDirector in VdsBrokerObjectsBuilder is an illustration for a > sub-optimal design: > 1. The methods it is used in them are checkTimeDrift and > reportInvalidInterfacesForNetwork. Both are: > public static and both should not be > Best regards, > ____________________ > Yevgeny Zaspitsky > Senior Software Engineer > Red Hat Israel > 34 Jerusalem Road, > Ra'anana, Israel 43501 > Tel: +972 9 7692098 > Mobile: +972 52 6323656 > Email: yzasp...@redhat.com > IRC: yzaspits > ----- Original Message ----- > > From: "Yair Zaslavsky" <yzasl...@redhat.com> > > > To: "Yevgeny Zaspitsky" <yzasp...@redhat.com> > > > Cc: devel@ovirt.org > > > Sent: Monday, October 6, 2014 5:23:25 PM > > > Subject: Re: [ovirt-devel] [ENGINE] splitting AuditLogDirector to its own > > module > > > ----- Original Message ----- > > > > From: "Yevgeny Zaspitsky" <yzasp...@redhat.com> > > > > To: "Yair Zaslavsky" <yzasl...@redhat.com>, devel@ovirt.org > > > > Sent: Sunday, October 5, 2014 11:31:18 AM > > > > Subject: Re: [ovirt-devel] [ENGINE] splitting AuditLogDirector to its own > > > module > > > > > > > > Hi Yair and All, > > > > > > > > As soon as you turned our attention to dependencies problems in oVirt, I > > > > guess would worse to mention that vdsbroker is directly dependent on dal > > > > (very weird to me). > > > > That is because some business logic found its way into vdsbroker code > > > > and uses dal directly. Using dal should be allowed to bll module only. > > > > Next to allowing ourselves using dal in vdsbroker, we've found very > > > > creative way to use bll commands in vdsbroker completing by that a > > > > circular dependency ring. > > > > > > > > The current situation when vdsbroker is directly dependent on dal and in > > > > fact dependent on bll is sever violation of a good design practice IMHO. > > > > > > > > I'm in favor solving that dependency mesh asap prior it become more > > > > difficult. We should strive to extract the business logic from vdsbroker > > > > (and any other module) and move that to bll module where its natural > > > > place is, then fix the dependencies chain (that have to be a tree and > > > > not any geometric figure). > > > > > > > > When that will happen, we'd be able to figure more easily what the > > > > natural place for AuditLogDirector should be, I guess that'll be utils > > > > package. > > > > > > > > Regards, > > > > Yevgeny > > > git grep will show you that it is in VdsBrokerObjectsBuilder which creates > > the entities from the XMlRpcStruct maps. > > > The calls to AuditLogDirector simply "report" of various issues during the > > "transformation" to the audit log, to be presented at UI. The objects > > builder class already involves business entities. > > > I agre usage of AuditLogDirector there seems ackward. > > > > > > > > On 03/10/14 20:03, Yair Zaslavsky wrote: > > > > > Hi all, > > > > > I just reviewed a patch regarding AuditLogDirector and I thought to > > > > myself > > > > > - why is it located in DAL? > > > > > Our maven dependencies are built in a way that both bll and vdsbroker > > > > > depend on DAL, and if you perform git grep AuditLogDirector you will > > > > see > > > > > it is used > > > > > by bll and vdsbroker. > > > > > But that this is not necessary a reason to include AuditlogDirector in > > > > DAL. > > > > > I think DAL should be our Data Access Layer, and for audit log (which > > > > is > > > > a > > > > > class that uses our DAOs) we should have a separate module. > > > > > > > > > > Thoughts on this are welcome, > > > > > > > > > > Yair > > > > > _______________________________________________ > > > > > Devel mailing list > > > > > Devel@ovirt.org > > > > > http://lists.ovirt.org/mailman/listinfo/devel > > > > > > > > >
_______________________________________________ Devel mailing list Devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/devel