------=_Part_15572334_1492903721.1412620157927
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
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(a)redhat.com
IRC: yzaspits
----- Original Message -----
From: "Yevgeny Zaspitsky" <yzaspits(a)redhat.com>
To: "Yair Zaslavsky" <yzaslavs(a)redhat.com>
Cc: devel(a)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(a)redhat.com
IRC: yzaspits
----- Original Message -----
> From: "Yair Zaslavsky" <yzaslavs(a)redhat.com>
> To: "Yevgeny Zaspitsky" <yzaspits(a)redhat.com>
> Cc: devel(a)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(a)redhat.com>
> > To: "Yair Zaslavsky" <yzaslavs(a)redhat.com>, devel(a)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(a)ovirt.org
> > >
http://lists.ovirt.org/mailman/listinfo/devel
> >
> >
------=_Part_15572334_1492903721.1412620157927
Content-Type: text/html; charset=utf-8
Content-Transfer-Encoding: quoted-printable
<html><body><div style=3D"font-family: times new roman, new york,
times, se=
rif; font-size: 12pt; color: #000000"><div>Please ignore my previous-mail,
=
that was sent accidentally.</div><div> <br></div><div>Usage
of AuditLogDire=
ctor in VdsBrokerObjectsBuilder is an illustration for a sub-optimal design=
:</div><div><ol><li>The methods it is used in them are
checkTimeDrift and r=
eportInvalidInterfacesForNetwork. Both
are:</li><ol><li>static</li><li>shou=
ld not be part of the class they are in<br></li><li>implement a piece of
bu=
siness logic that should not be in vdsbroker, but in
bll<br></li></ol><li>v=
dsbroker module should do the "transformation" and that only. Any other log=
ic is a business one and should be in
bll.<br></li></ol></div><div><br></di=
v><div><span name=3D"x"></span>Best regards,
<br>____________________ <br>Y=
evgeny Zaspitsky <br>Senior Software Engineer <br>Red Hat Israel <br>34
Jer=
usalem Road, <br>Ra'anana, Israel 43501
<br><div><br></div>Tel: +972 9 7692=
098 <br>Mobile: +972 52 6323656 <br>Email: yzaspits(a)redhat.com <br>IRC:
yza=
spits <br><span
name=3D"x"></span><br></div><div><br></div><hr
id=3D"zwchr"=
<blockquote style=3D"border-left:2px solid
#1010FF;margin-left:5px;padding=
-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:n=
one;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><b>From:
</b>"Y=
evgeny Zaspitsky" &lt;yzaspits(a)redhat.com&gt;<br><b>To:
</b>"Yair Zaslavsky=
" &lt;yzaslavs(a)redhat.com&gt;<br><b>Cc:
</b>devel(a)ovirt.org<br><b>Sent: </b=
Monday, October 6, 2014 9:19:31 PM<br><b>Subject:
</b>Re: [ovirt-devel] [E=
NGINE] splitting AuditLogDirector to its own
module<br><div><br></div><div =
style=3D"font-family: times new roman, new york, times, serif; font-size: 1=
2pt; color: #000000"><div><span style=3D"font-family:
arial,helvetica,sans-=
serif;">Usage of AuditLogDirector in VdsBrokerObjectsBuilder is an illustra=
tion for a sub-optimal design:</span><br><span style=3D"font-family:
arial,=
helvetica,sans-serif;">1. The methods it is used in them are checkTimeDrift=
and reportInvalidInterfacesForNetwork. Both are:</span><br><span
style=3D"=
font-family: arial,helvetica,sans-serif;">public static and both should not=
be</span><br></div><div><br></div><div><span
style=3D"font-family: arial,h=
elvetica,sans-serif;"><span></span>Best regards,
</span><br><span style=3D"=
font-family: arial,helvetica,sans-serif;">____________________
</span><br><=
span style=3D"font-family: arial,helvetica,sans-serif;">Yevgeny Zaspitsky
<=
/span><br><span style=3D"font-family:
arial,helvetica,sans-serif;">Senior S=
oftware Engineer </span><br><span style=3D"font-family:
arial,helvetica,san=
s-serif;">Red Hat Israel </span><br><span
style=3D"font-family: arial,helve=
tica,sans-serif;">34 Jerusalem Road, </span><br><span
style=3D"font-family:=
arial,helvetica,sans-serif;">Ra'anana, Israel 43501
</span><br><div><br></=
div><span style=3D"font-family: arial,helvetica,sans-serif;">Tel: +972
9 76=
92098 </span><br><span style=3D"font-family:
arial,helvetica,sans-serif;">M=
obile: +972 52 6323656 </span><br><span style=3D"font-family:
arial,helveti=
ca,sans-serif;">Email: yzaspits(a)redhat.com </span><br><span
style=3D"font-f=
amily: arial,helvetica,sans-serif;">IRC: yzaspits
</span><br><span></span><=
br></div><div><br></div><hr
id=3D"zwchr"><blockquote style=3D"border-left:2=
px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:no=
rmal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,san=
s-serif;font-size:12pt;"><b>From: </b>"Yair Zaslavsky"
&lt;yzaslavs(a)redhat.=
com><br><b>To: </b>"Yevgeny Zaspitsky"
&lt;yzaspits(a)redhat.com&gt;<br><b=
Cc: </b>devel(a)ovirt.org<br><b>Sent:
</b>Monday, October 6, 2014 5:23:25 PM=
<br><b>Subject:
</b>Re: [ovirt-devel] [ENGINE] splitting AuditLogDirector t=
o its own
module<br><div><br></div><br><div><br></div><hr
id=3D"zwchr"><br>=
> From: "Yevgeny Zaspitsky"
&lt;yzaspits(a)redhat.com&gt;<br>&gt; To: "Yai=
r Zaslavsky" &lt;yzaslavs(a)redhat.com&gt;, devel(a)ovirt.org<br>&gt;
Sent: Sun=
day, October 5, 2014 11:31:18 AM<br>> Subject: Re: [ovirt-devel] [ENGINE=
] splitting AuditLogDirector to its own module<br>> <br>> Hi
Yair and=
All,<br>> <br>> As soon as you turned our attention to
dependencies =
problems in oVirt, I<br>> guess would worse to mention that vdsbroker is=
directly dependent on dal<br>> (very weird to me).<br>> That is
beca=
use some business logic found its way into vdsbroker code<br>> and uses =
dal directly. Using dal should be allowed to bll module only.<br>> Next =
to allowing ourselves using dal in vdsbroker, we've found very<br>>
crea=
tive way to use bll commands in vdsbroker completing by that a<br>> circ=
ular dependency ring.<br>> <br>> The current situation when
vdsbroker=
is directly dependent on dal and in<br>> fact dependent on bll is sever=
violation of a good design practice IMHO.<br>> <br>> I'm in
favor so=
lving that dependency mesh asap prior it become more<br>> difficult. We =
should strive to extract the business logic from vdsbroker<br>> (and any=
other module) and move that to bll module where its natural<br>> place =
is, then fix the dependencies chain (that have to be a tree and<br>> not=
any geometric figure).<br>> <br>> When that will happen,
we'd be abl=
e to figure more easily what the<br>> natural place for AuditLogDirector=
should be, I guess that'll be utils<br>> package.<br>>
<br>> Rega=
rds,<br>> Yevgeny<br><div><br></div>git grep will
show you that it is in=
VdsBrokerObjectsBuilder which creates the entities from the XMlRpcStruct m=
aps.<br>The calls to AuditLogDirector simply "report" of various issues
dur=
ing the "transformation" to the audit log, to be presented at UI. The objec=
ts builder class already involves business entities.<br>I agre usage of Aud=
itLogDirector there seems
ackward.<br><div><br></div><br>> <br>>
On 0=
3/10/14 20:03, Yair Zaslavsky wrote:<br>> > Hi all,<br>>
> I ju=
st reviewed a patch regarding AuditLogDirector and I thought to myself<br>&=
gt; > - why is it located in DAL?<br>> > Our maven dependencies
ar=
e built in a way that both bll and vdsbroker<br>> > depend on DAL,
an=
d if you perform git grep AuditLogDirector you will see<br>> > it is
=
used<br>> > by bll and vdsbroker.<br>> > But that
this is not n=
ecessary a reason to include AuditlogDirector in DAL.<br>> > I think
=
DAL should be our Data Access Layer, and for audit log (which is a<br>> =
> class that uses our DAOs) we should have a separate module.<br>>
&g=
t;<br>> > Thoughts on this are welcome,<br>>
><br>> > Yai=
r<br>> >
_______________________________________________<br>> >=
Devel mailing list<br>> > Devel(a)ovirt.org<br>&gt; >
http://lists.=
ovirt.org/mailman/listinfo/devel<br>> <br>>
<br></blockquote><div><br=
</div></div></blockquote><div><br></div></div></body></html>
------=_Part_15572334_1492903721.1412620157927--