geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From John Blum <jb...@pivotal.io>
Subject org.apache.geode.internal.InternalDataSerializer API Changes...
Date Wed, 17 May 2017 23:42:16 GMT
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/a44585316a8d7eed35917c1dfd829315b34ce316/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/61f676b0187e8918ca62f4a2ec9b6bd3baa942d3/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/a44585316a8d7eed35917c1dfd829315b34ce316/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/61f676b0187e8918ca62f4a2ec9b6bd3baa942d3/geode-core/src/main/java/org/apache/geode/cache/client/Pool.java#L210
[9] https://build.spring.io/browse/SGF-NAG-552

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message