[ovirt-devel] [ENGINE] splitting AuditLogDirector to its own module

Yevgeny Zaspitsky yzaspits at redhat.com
Mon Oct 6 18:29:17 UTC 2014


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: yzaspits at redhat.com 
IRC: yzaspits 

----- Original Message -----

> From: "Yevgeny Zaspitsky" <yzaspits at redhat.com>
> To: "Yair Zaslavsky" <yzaslavs at redhat.com>
> Cc: devel at 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: yzaspits at redhat.com
> IRC: yzaspits

> ----- Original Message -----

> > From: "Yair Zaslavsky" <yzaslavs at redhat.com>
> 
> > To: "Yevgeny Zaspitsky" <yzaspits at redhat.com>
> 
> > Cc: devel at 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" <yzaspits at redhat.com>
> 
> > > To: "Yair Zaslavsky" <yzaslavs at redhat.com>, devel at 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 at ovirt.org
> 
> > > > http://lists.ovirt.org/mailman/listinfo/devel
> 
> > >
> 
> > >
> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ovirt.org/pipermail/devel/attachments/20141006/ee81a980/attachment-0001.html>


More information about the Devel mailing list