Hi Kirk (also responding to Bruce as I was writing this before he sent)- Thank you for following up.
Regarding the addition of the getOnlineLocators()... SDG does implement <http://docs.spring.io/spring-data-gemfire/docs/current/api/org/springframework/data/gemfire/client/support/package-summary.html> [1] a few Geode interfaces, such as *org.apache.geode.cache.client.Pool*. This is due in part since not all the state for certain Geode objects (e.g. Pool) is known in advance as the actual object may not have been created by the *Spring* container yet. However, that state is captured in *Spring* FactoryBeans, which will be used to construct the actual object at the appropriate time. Anyway, like the "internal" API, I have tried to keep such implementations of Geode interfaces/classes to a minimum. Still, it is a good indicator of when something has changed since I will most certainly get a compilation failure. This leads me to, I do need to know when APIs change to provide appropriate support in SDG, possibly in configuration, whether XML of via Java/annotations with the FactoryBeans, e.g. PoolFactoryBean <http://docs.spring.io/spring-data-gemfire/docs/current/api/org/springframework/data/gemfire/client/PoolFactoryBean.html> [2], especially if it is a configuration property/attribute. Other cases my be important from an accessibility or information standpoint (probably more so for tooling). As for the InternalDataSerializer... No worries. I found an acceptable and actually better workaround. Again, I am trying to reduce or completely eliminate the use of "internal" classes in SDG. Unfortunately, a quick search (using regex) in SDG for "import org\.apache\.geode.*internal" reveals 35 occurrences, the most pertinent ones being... org.apache.geode.internal.GemFireVersion (used in debugging) org.apache.geode.internal.DistributionLocator org.apache.geode.internal.InternalDataSerializer org.apache.geode.internal.cache.GemFireCacheImpl org.apache.geode.internal.cache.LocalRegion (used to inspect whether a server proxy is present to acquire the appropriate QueryService) org.apache.geode.internal.cache.PartitionedRegion org.apache.geode.internal.cache.UserSpecifiedRegionAttributes org.apache.geode.internal.cache.lru.LRUCapacityController org.apache.geode.internal.cache.lru.MemLRUCapacityController org.apache.geode.internal.cache.query.internal.ResultsBag org.apache.geode.internal.distributed.internal.DistributionConfig org.apache.geode.internal.distributed.internal.InternalDistributedSystem org.apache.geode.internal.distributed.internal.InternalLocator org.apache.geode.internal.datasource.ConfigProperty (JNDI support) org.apache.geode.internal.jndi.JNDIInvoker (JNDI support) org.apache.geode.internal.security.SecurityService (my doing) Most of this was added before my time. I just need time to review these more carefully, and how each is used. I know many of these are used for good reason. Thanks, John [1] http://docs.spring.io/spring-data-gemfire/docs/current/api/org/springframework/data/gemfire/client/support/package-summary.html [2] http://docs.spring.io/spring-data-gemfire/docs/current/api/org/springframework/data/gemfire/client/PoolFactoryBean.html On Thu, May 18, 2017 at 10:18 AM, Bruce Schuchardt <bschucha...@pivotal.io> wrote: > John, is it possible to list the internal APIs needed by SDG? I don't > think we can live with a requirement that all internal API changes need to > be communicated and blessed. We need to address SDG's needs in the public > APIs and get rid of this dependency. > > > Le 5/18/2017 à 9:36 AM, Kirk Lund a écrit : > >> Hi John! How did the addition of getOnlineLocators() cause SDG build to >> fail? I checked [9] but I couldn't quite grok what happened. I would've >> thought that adding would be ok but removing could potentially break SDG. >> >> The InternalDataSerializer change was probably mine. I reduced scope of >> methods and variables where possible. Sorry about that. Do you need me to >> restore getSerializer to public? >> >> On Wed, May 17, 2017 at 4:42 PM, John Blum <jb...@pivotal.io> wrote: >> >> Geode devs- >>> >>> I am not sure how decisions are made when changing Geode's API, but I >>> would >>> caution that more care should be taken when doing so, regardless of >>> whether >>> the APIs are public or not, but especially when they are public. >>> >>> Unfortunately, in this particular case, the API in question is deemed >>> "internal", which I know, are subject to change. However, the problem >>> with >>> this is, the public API is insufficient in some cases, particularly for >>> "frameworks" (e.g. SDG) and "tooling" building on Geode and attempting to >>> uphold Geode's functional/behavioral contract. >>> >>> By way of example, a recent *Spring Data Geode* build broke >>> <https://build.spring.io/browse/SGF-NAG-556/log> [1] because, the >>> org.apache.geode.internal.InternalDataSerializer class was recently >>> changed. The scope of getSerializer(:Class):DataSerializer was changed >>> from *public >>> <https://github.com/apache/geode/blob/rel/v1.1.1/geode- >>> core/src/main/java/org/apache/geode/internal/InternalDataSer >>> ializer.java# >>> L1113>* >>> [2] >>> to *private >>> <https://github.com/apache/geode/blob/a44585316a8d7eed35917c1dfd8293 >>> 15b34ce316/geode-core/src/main/java/org/apache/geode/internal/ >>> InternalDataSerializer.java#L1090>* >>> [3] >>> in a seemingly unrelated commit. >>> >>> There is a class >>> <https://github.com/spring-projects/spring-data-geode/ >>> blob/master/src/main/java/org/springframework/data/gemfire/ >>> serialization/EnumSerializer.java#L39> >>> [4] >>> in SDG that extends DataSerializer >>> <http://gemfire-90-javadocs.docs.pivotal.io/org/apache/ >>> geode/DataSerializer.html> >>> [5] >>> (in Geode's public API) but must use the InternalDataSerializer (internal >>> Geode API) when changes (e.g. "supported classes") to the SDG serializer >>> need to be "distributed" across the cluster. SDG"s serializer use to >>> call >>> this getSerializer(:Class):DataSerializer method. However, in this >>> case, >>> I >>> fixed the problem by not calling this "overloaded" method as it was not >>> strictly necessary. >>> >>> While I am trying to avoid the use of "internal" Geode classes and APIs >>> in >>> SDG in general, as I mention above, this is not always possible. For >>> instance, there are a few API blunders such as the ability to register >>> <http://gemfire-90-javadocs.docs.pivotal.io/org/apache/ >>> geode/DataSerializer.html#register-java.lang.Class-> >>> [6] >>> a DataSerializer class but not "unregister" it; that is in >>> <https://github.com/apache/geode/blob/rel/v1.1.1/geode- >>> core/src/main/java/org/apache/geode/internal/InternalDataSer >>> ializer.java# >>> L1077> >>> [7] >>> InternalDataSerializer. The other bad API practices on this class (i.e. >>> (Internal)DataSerializer) alone. >>> >>> Framework and tool developers must occasionally rely on "internal" APIs >>> (especially when the public API is insufficient) to order to achieve a >>> similar experience to Geode alone; something to keep in mind as Geode's >>> ecosystem grows. >>> >>> Finally, I'll also add that, I do need to know anytime the public API >>> changes as well. Recently the getOnlineLocators() method >>> <https://github.com/apache/geode/blob/61f676b0187e8918ca62f4a2ec9b6b >>> d3baa942d3/geode-core/src/main/java/org/apache/geode/ >>> cache/client/Pool.java#L210> >>> [8] >>> was added to org.apache.geode.cache.client.Pool, which caused another >>> SDG >>> build <https://build.spring.io/browse/SGF-NAG-552> [9] to fail. >>> >>> Special thank you to *Nabarun* and *Diane* for the recent heads about the >>> LuceneQueryFactory method name change... from setResultLimit(:int) to >>> setLimit(:int). >>> >>> Regards, >>> >>> -- >>> -John >>> >>> >>> [1] https://build.spring.io/browse/SGF-NAG-556/log >>> [2] >>> https://github.com/apache/geode/blob/rel/v1.1.1/geode- >>> core/src/main/java/org/apache/geode/internal/InternalDataSer >>> ializer.java# >>> L1113 >>> [3] >>> https://github.com/apache/geode/blob/a44585316a8d7eed35917c1dfd8293 >>> 15b34ce316/geode-core/src/main/java/org/apache/geode/internal/ >>> InternalDataSerializer.java#L1090 >>> [4] >>> https://github.com/spring-projects/spring-data-geode/ >>> blob/master/src/main/java/org/springframework/data/gemfire/ >>> serialization/EnumSerializer.java#L39 >>> [5] >>> http://gemfire-90-javadocs.docs.pivotal.io/org/apache/ >>> geode/DataSerializer.html >>> [6] >>> http://gemfire-90-javadocs.docs.pivotal.io/org/apache/ >>> geode/DataSerializer.html#register-java.lang.Class- >>> [7] >>> https://github.com/apache/geode/blob/rel/v1.1.1/geode- >>> core/src/main/java/org/apache/geode/internal/InternalDataSer >>> ializer.java# >>> L1077 >>> [8] >>> https://github.com/apache/geode/blob/61f676b0187e8918ca62f4a2ec9b6b >>> d3baa942d3/geode-core/src/main/java/org/apache/geode/ >>> cache/client/Pool.java#L210 >>> [9] https://build.spring.io/browse/SGF-NAG-552 >>> >>> > -- -John john.blum10101 (skype)
