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/InternalDataSerializer.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/InternalDataSerializer.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/InternalDataSerializer.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/InternalDataSerializer.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 >
