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
Re: [ovirt-devel] [ENGINE] splitting AuditLogDirector to its own module
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
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
[ovirt-devel] [ENGINE] splitting AuditLogDirector to its own module
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
