geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Galen O'Sullivan" <gosulli...@pivotal.io>
Subject Re: [DISCUSS] Geode packages mustn't span Jigsaw modules
Date Mon, 26 Nov 2018 21:05:49 GMT
>
> If you want to
> maintain an API as internal, then you should properly protect that API with
> the appropriate *Java* access modifiers (private, package-private and
> protected).
>

Hopefully modules will allow us to do this better, by restricting which
*classes* are visible outside the module. However, Java doesn't have a
concept of submodules, which means there's no way to make
org.apache.geode.cache.tier.sockets visible to org.apache.cache or
org.apache.cache.tier without also being visible to everything. We can't
make InternalCache (for example) only visible to our own code without
declaring it package-private and putting (effectively) all of Geode in the
same package.

Adding a method to
> an interface, is an API breaking change.
>
I don't agree with this. If a new method is added, old API works just fine,
and unless I'm mistaken (and I may be), any code that used the old
interface should be able to use the new interface without issues.

Galen

On Mon, Nov 26, 2018 at 12:13 PM John Blum <jblum@pivotal.io> wrote:

> *> Most of these are in internal packages, which means we can change the
> package of these classes without breaking users.*
>
> I don't agree with this.
>
> Some functionality in Geode required by an application, or framework, is
> only available in an "internal" package, unfortunately.  This has to do
> with the fact that many interfaces in Geode were not well designed.
>
> In general, and as a general rule of thumb, anytime you change a method
> name or method signature, it is an API breaking change.  Adding a method to
> an interface, is an API breaking change.  Removing methods from an
> interface is (potentially) an API breaking change.  Changing a type is an
> API breaking change.  There are several other examples.  Any class/method
> (dare I say field of a class) that is not "properly protected" is part of
> your API, regardless if we deem it be "internal" only or not, and
> (possibly) will affect users.
>
> I think any level of modularity (and maturing) is pointless without 1)
> first, having well defined contracts/interfaces between functional areas
> (based on concerns) and 2) using appropriate access modifiers and cleanly
> delineating APIs/SPIs from implementations/providers.
>
> -j
>
>
>
> On Mon, Nov 19, 2018 at 5:21 PM Dan Smith <dsmith@pivotal.io> wrote:
>
> > I think we can actually fix most of these issues without geode-2.0. Most
> of
> > these are in internal packages, which means we can change the package of
> > these classes without breaking users. The only concerning one is
> > org.apache.geode.cache.util, which is a public package. However, the
> > AutoBalancer is actually still marked @Experimental, so we could
> > technically move that as well. Or maybe deprecate the old one and it's
> > associated jar, and create a new jar with a reasonable package. JDK11
> users
> > could just avoid the old jar.
> >
> > I have been wondering for a while if we should just fold geode-cq and
> > geode-wan back into geode-core. These are not really properly separated
> > out, they are very tangled with core. The above package overlap kinda
> shows
> > that as well. But maybe going through the effort of renaming the packages
> > to make them java 11 modules would help improve the code anyway!
> >
> > -Dan
> >
> >
> >
> >
> > On Mon, Nov 19, 2018 at 5:03 PM Owen Nichols <onichols@pivotal.io>
> wrote:
> >
> > > To package Geode as Java 11 Jigsaw module(s), a major hurdle is the
> > > requirement that packages cannot be split across modules.
> > >
> > > If we simply map existing Geode jars to Modules, we have about 10 split
> > > packages (see table below).  Any restructuring would potentially have
> to
> > > wait until Geode 2.0.
> > >
> > > A workaround would be to map existing packages into a small number of
> new
> > > modules (e.g. geode-api and geode-internal).  Existing jars would
> remain
> > > as-is.  Users making the transition to Java 11 /w Jigsaw already need
> to
> > > switch from CLASSPATH to MODULEPATH  so the inconvenience of a
> different
> > > naming scheme for Geode-as-modules might be acceptable (however, once
> > > module naming and organization is established, we may be stuck with it
> > for
> > > a long time).
> > >
> > > Thoughts?
> > >
> > > Is it even possible to rearrange all classes in each package below
> into a
> > > single jar?  Is doing so desirable enough to defer Java 11 support
> until
> > a
> > > yet-unplanned Geode 2.0, or perhaps to drive such a release?
> > >
> > > Or, is it satisfactory to fatten Geode releases to include one set of
> > jars
> > > for CLASSPATH use, plus a different set of jars for MODULEPATH use?
> > >
> > >
> > > Package
> > > Jar
> > > org.apache.geode.cache.client.internal
> > > geode-core-1.8.0.jar
> > > geode-cq-1.8.0.jar
> > > geode-wan-1.8.0.jar
> > > org.apache.geode.cache.client.internal.locator.wan
> > > geode-core-1.8.0.jar
> > > geode-wan-1.8.0.jar
> > > org.apache.geode.cache.query.internal.cq
> > > geode-core-1.8.0.jar
> > > geode-cq-1.8.0.jar
> > > org.apache.geode.cache.util
> > > geode-core-1.8.0.jar
> > > geode-rebalancer-1.8.0.jar
> > > org.apache.geode.internal
> > > geode-connectors-1.8.0.jar
> > > geode-core-1.8.0.jar
> > > geode-cq-1.8.0.jar
> > > geode-lucene-1.8.0.jar
> > > geode-wan-1.8.0.jar
> > > org.apache.geode.internal.cache.tier.sockets.command
> > > geode-core-1.8.0.jar
> > > geode-cq-1.8.0.jar
> > > org.apache.geode.internal.cache.wan
> > > geode-core-1.8.0.jar
> > > geode-wan-1.8.0.jar
> > > org.apache.geode.internal.cache.wan.parallel
> > > geode-core-1.8.0.jar
> > > geode-wan-1.8.0.jar
> > > org.apache.geode.internal.cache.wan.serial
> > > geode-core-1.8.0.jar
> > > geode-wan-1.8.0.jar
> > > org.apache.geode.internal.protocol.protobuf.v1
> > > geode-protobuf-1.8.0.jar
> > > geode-protobuf-messages-1.8.0.jar
> >
>
>
> --
> -John
> john.blum10101 (skype)
>

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