*@Jared* - yes, makes sense. Thanks for the reference to FCS. I am familiar with it and I am also aware that Geode is starting to use this in other capacities.
*Regarding name...* The one thing I dislike about the name "RegisterFunction" is it implies some action will be performed by Geode (in general) just by the addition of the annotation. However, this is simply not true. AFAIK (or have heard so far) this annotation only works when "deploying" Functions via *Gfsh* in order to limit which classes loaded by the scan, which won't happen in any other context. As such, unfortunately (IMO) it is very context driven and dependent. Consider this... If a developer creates an "embedded" Geode "peer cache" application connected to the cluster, bundles it up in a JAR file that contains " RegisterFunction" annotated *Function* classes, and starts it up (e.g. `java -jar peerCacheApplication.jar`) Geode is not going to recognize the presence of this annotation on classes in the JAR. The developer cannot deploy the entire JAR (i.e. `deploy jar`) and expect the application to be run (e.g. *Initializer* style [1]) and so is likely to separate out (perhaps in another JAR) the *Functions* and deploy them independently, breaking encapsulation (microservices-style). Of course, *Gfsh* and the *Manager* do not require that *Functions* be explicitly broken out, but imagine deploying a "FAT" JAR (possibly containing all the *libs* the application requires at runtime). That most certainly will be much more expensive in terms of network bandwidth, particularly where *Cluster Config* is concerned. I only mention this since some frameworks/containers do support Annotations that "do affect the configuration" just by the mere presence of the annotation, which usually, but not always, is accompanied by some additional configuration that the framework/container will apply, every single time, not just based on tools. By way of example, using SDG, have a look at the @EnableSecurity [2] annotation and then look at @EnablePdx [3]. @EnableSecurity provides additional configuration for the container to apply where as @EnablePdx triggers "other" configuration supplied by SDG to the container. In both cases, the mere presence of the annotation(s) makes those things happen. Therefore, I concluded that @RegisterableFunction is more intuitive since it implies the *Function* "can be" registered, but at least to me, it also implies that the *Function* is not necessarily being registered. Regards, John [1] http://geode.apache.org/docs/guide/12/basic_config/the_cache/setting_cache_initializer.html [2] https://github.com/spring-projects/spring-data-geode/blob/2.0.0.RC2/src/main/java/org/springframework/data/gemfire/config/annotation/EnableSecurity.java#L50-L51 [3] https://github.com/spring-projects/spring-data-geode/blob/2.0.0.RC2/src/main/java/org/springframework/data/gemfire/config/annotation/EnablePdx.java#L40-L45 On Fri, Aug 11, 2017 at 4:16 PM, Kirk Lund <kl...@apache.org> wrote: > I can't even say "RegisterableFunction" flibble flobble ;) > > On Fri, Aug 11, 2017 at 4:09 PM, Jared Stewart <jstew...@pivotal.io> > wrote: > > > Hi John, > > > > Thanks for the suggestions. I like “@RegisterableFunction” better than > > “@RegisterFunction". I think that we don’t want @RegisterableFunction to > > be @Inherited, in order to avoid creating a new variant of the problem we > > are trying to fix. As you suggest, we should be mindful to document the > > behavior clearly. > > > > In case you’re interested, here’s <https://github.com/lukehutch/ > > fast-classpath-scanner/wiki/3.4.-Finding-classes-with- > > specific-annotations-or-meta-annotations> some documentation of the API > > for the library we’re using for the annotation scanning. It scans the > > binary byte code files directly, rather than using a Class reference like > > Spring utilities you pointed out (which AFAIK requires having that class > > loaded in the JVM). > > > > Thanks, > > Jared > > > > > On Aug 11, 2017, at 11:00 AM, John Blum <jb...@pivotal.io> wrote: > > > > > > Hi Jared- > > > > > > In general, I like this idea since Annotations are a great form of > > > meta-data and essentially meaningless outside of the intended context > and > > > therefore do not impose any adverse effects on any existing behavior. > > > > > > However, 2 things... 1 suggestion and 1 caution... > > > > > > > > > 1. Perhaps "RegisterableFunction" as opposed to "RegisterFunction". > > > > > > > > > 2. Annotations can be "inherited" in the same way that your > > ConcreteFunction > > > extends (inherits from) AbstractFunction, so too can a more concrete > > > Function possibly inherit from a less-concrete-but-not-abstract > Function. > > > A developer might expect that they don't have to re-annotated his/her > > > function. > > > > > > For example... > > > > > > @RegisterableFunction > > > class ProcessOrder extends FunctionAdapter { ... } > > > > > > class CreateAccountAtPointOfSale extends ProcessOrder { ... } > > > > > > If these are in separate JAR files, depending on the "application", I > may > > > only want to "register" the CreateAccountAtPointOfSale Function and not > > the > > > ProcessOrder Function explicitly. Please forgive my example here since > > it > > > seems like a bad example given it feels like 2 separate actions but is > > > often part of the same workflow and so really depends on how > application > > > developers think about and organize their logic, which usually leads to > > > "unexpected" things you never anticipated when introducing something > like > > > this. > > > > > > SIDE NOTE: Of course, from a Java SE standpoint, the > "RegisteredFunction" > > > Annotation could be (but does not have to be) meta-annotated with > > @Inherited., > > > such as... > > > > > > @Target(ElementType.TYPE) > > > @Retention(RetentionPolicy.RUNTIME) > > > @Inherited > > > ... > > > @interface RegisteredFunction { .. } > > > > > > > > > *Spring* takes all of these things into account when it processes > > > annotations of this nature (e.g. @Enable...). Especially have a look > at > > > o.s.core.annotation.AnnotatedElementUtils [1] and even > > > o.s.core.annotation.AnnotationUtils [2]. These are very robust and > very > > > powerful classes underpinning much of *Spring's* Annotation > configuration > > > and processing. *Boot* also extends this functionality and *Spring* > > > Annotation config in very specific/custom ways. > > > > > > I think it is reasonable to set limitations to keep the initial scope > > > small, but be sure those are well documented since users will be coming > > > from many different frameworks having many different expectations. > > > > > > Food for thought/hope this helps. > > > > > > Regards, > > > -John > > > > > > > > > [1] > > > http://docs.spring.io/spring/docs/current/javadoc-api/org/ > > springframework/core/annotation/AnnotatedElementUtils.html > > > [2] > > > http://docs.spring.io/spring/docs/current/javadoc-api/org/ > > springframework/core/annotation/AnnotationUtils.html > > > > > > > > > > > > > > > > > > On Fri, Aug 11, 2017 at 10:22 AM, Jared Stewart <jstew...@pivotal.io> > > wrote: > > > > > >> Recent changes introduced to avoid the eager loading of classes have > > lead > > >> to Functions not getting registered correctly if the class hierarchy > > >> leading up to Function (or FunctionAdapter) is split up across > multiple > > jar > > >> files. We propose to introduce a new annotation to identify functions > > in > > >> such a case. > > >> Consider the following scenario: > > >>> Abstract.jar - public abstract class AbstractFunction implements > > >> Function {...} > > >>> Concrete.jar - public class ConcreteFunction extends AbstractFunction > > >> {...} > > >> When Concrete.jar is deployed, we only scan the classes inside > > >> Concrete.jar. This means that we have no way of knowing that > > >> AbstractFunction eventually leads up to Function. (We could load > > >> ConcreteFunction to see if it implements Function via reflection or > > >> Function.class.isAssignableFrom(), but then we would be back to > eagerly > > >> loading all of the classes to see whether or not they implement > > Function.) > > >> We propose a new annotation (perhaps @RegisterFunction, suggestions > are > > >> welcome) to designate a class as a Function in such a case. Since we > > are > > >> able to scan classes for annotations without loading them, this will > > allow > > >> us to identify ConcreteFunction as a Function without eagerly loading > > all > > >> of the classes in Concrete.jar. > > >> I should emphasize that ConcreteFunction already is registered as > > expected > > >> if it resides in the same jar file as AbstractFunction. This > annotation > > >> would only be relevant when the class hierarchy leading up to Function > > is > > >> spread across multiple jar files. > > >> - Jared > > > > > > > > > > > > > > > -- > > > -John > > > john.blum10101 (skype) > > > > > -- -John john.blum10101 (skype)
