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


Reply via email to