----- Original Message -----
> From: "Tomas Jelinek" <[email protected]>
> To: "Vojtech Szocs" <[email protected]>
> Cc: "devel" <[email protected]>
> Sent: Monday, December 19, 2016 7:50:55 PM
> Subject: Re: [ovirt-devel] Using SLF4J in frontend code
> 
> On Mon, Dec 19, 2016 at 6:26 PM, Vojtech Szocs <[email protected]> wrote:
> 
> >
> >
> > ----- Original Message -----
> > > From: "Tomas Jelinek" <[email protected]>
> > > To: "Vojtech Szocs" <[email protected]>
> > > Cc: "devel" <[email protected]>
> > > Sent: Thursday, December 15, 2016 8:27:10 AM
> > > Subject: Re: [ovirt-devel] Using SLF4J in frontend code
> > >
> > > On Wed, Dec 14, 2016 at 5:27 PM, Vojtech Szocs <[email protected]>
> > wrote:
> > >
> > > > Hi,
> > > >
> > > > I was looking into "why" we need to emulate SLF4J classes for usage
> > > > in GWT frontend (given that one should use java.util.logging instead):
> > > >
> > > >   frontend/webadmin/modules/gwt-extension/src/main/java/org/
> > > > ovirt/engine/ui/uioverrides/org/slf4j/Logger.java
> > > >   frontend/webadmin/modules/gwt-extension/src/main/java/org/
> > > > ovirt/engine/ui/uioverrides/org/slf4j/LoggerFactory.java
> > > >
> > > > I found two frontend-specific classes, which I fixed. However, I could
> > > > not remove ^^ SLF4J class overrides, because some shared (backend) code
> > > > still uses SLF4J, in particular:
> > > >
> > > >   VmRngDevice.java => backend, common module
> > > >   EnumValueAutoCompleter.java => backend, searchbackend module
> > > >   SyntaxChecker.java => backend, searchbackend module
> > > >
> > > > 1, should a backend business entity (VmRngDevice) perform logging?
> > > >
> > >
> > > Well, VmRngDevice has some data and has some logic. And since it has
> > logic,
> > > I think it should also be allowed to
> > > do some logging.
> > >
> > > I honestly don't have issues with entities to have some degree of logic,
> > > the problem is that they are sent to FE.
> >
> > If we adhere to existing backend design, all business logic should
> > be isolated into dedicated commands. This implies leaving entities
> > void of actual business logic.
> >
> > But VmRngDevice#csvToSourcesSet isn't really business logic, it's
> > just a utility to parse String into Set<Source>.
> 
> 
> Lets not start a discussion if this is a logic or business logic or utility
> function or whatever kind of logic :)
> 
> So, what are you proposing? Extract it to a utility class?

Yes, ideally, we'd extract it to a class which doesn't belong into
any "visible by GWT" package [1]:

  org.ovirt.engine.core.(common|compat)

[1] see `gwt.dontPrune` in frontend/webadmin/modules/pom.xml

VmRngDevice#csvToSourcesSet is clearly a utility to parse a string
fetched from DB via DAOs [additional_rng_sources] or from host via
JSON RPC [VDSInfoReturn]; makes no sense for GWT compiler to see it.

> 
> 
> >
> 
> Just wondering why
> > are we suppressing IllegalArgumentException (parse errors) there.
> >
> 
> Because of this bug: https://bugzilla.redhat.com/show_bug.cgi?id=1374216
> Long story short, the host can return us a rng device we don't know about
> and we don't want to die on it.
> Could be implemented also in a nicer way, but it is correct...
> 
> > But this is an old decision and we need to live with it (for now). That
> > > particular log statement looks quite useful to me and I
> > > would not remove it.
> >
> > The thing is, VmRngDevice#csvToSourcesSet is only called on backend,
> > but since we tell GWT compiler that all business entity code is live
> > (via DontPrune aspect), we still need to emulate SLF4J logging API..
> >
> > >
> > >
> > > > 2, assuming "searchbackend" functionality is exposed only via the GWT
> > > >    frontend, why it's represented as a backend module?

Is it OK to move searchbackend under frontend/webadmin/modules?

If not, is it OK to remove org.slf4j.* imports in searchbackend
and replace them with java.util.logging.* imports?

Also, as searchbackend is currently in backend/manager/modules,
the searchbackend.jar is deployed to WildFly [2] even though it
isn't used by any backend code.. (only needed for GWT compile,
since the functionality is exposed only through GWT UI..)

[2] 
/usr/share/ovirt-engine/modules/common/org/ovirt/engine/core/searchbackend/main/searchbackend.jar

> > > >
> > > > Thanks,
> > > > Vojtech
> > > > _______________________________________________
> > > > Devel mailing list
> > > > [email protected]
> > > > http://lists.phx.ovirt.org/mailman/listinfo/devel
> > > >
> > >
> >
> 
_______________________________________________
Devel mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/devel

Reply via email to