I've sent in a pull request for this: https://github.com/apache/geode/pull/1119
I've added the old serialVersionUID to the FunctionAdapter, under the assumption that anyone in 1.0-1.3 would have seen that the FunctionAdapter had been deprecated and not used it. The 1.0-1.3 users could also easily change "extends FunctionAdapter" to "implements Function", recompile and things would work for them. The users pre-1.0 would have a slightly more difficult approach. Anyone interested, please review/accept or reject the pull request. Thanks, -Jason On Wed, Nov 29, 2017 at 9:09 AM Darrel Schneider <dschnei...@pivotal.io> wrote: > +1 to not removing deprecated apis in minor releases. > The semver policy Alexander describes seems reasonable. > In the past of we have had something deprecated for a long time we have > felt free to remove it whenever we want but I think the semver policy is a > better way to decide when we are free to remove deprecated external apis. > > > On Wed, Nov 29, 2017 at 8:50 AM, Alexander Murmann <amurm...@pivotal.io> > wrote: > > > Even though the class is deprecated, you should be able to go from one > > minor version to another without having to worry about anything breaking. > > The point of semver is to provide information about things breaking or > not > > without having to read the changelog. If we remove APIs in a minor > version > > because they were previously deprecated we break that contract. > Semver.org > > <https://semver.org/#how-should-i-handle-deprecating-functionality> > > recommends > > that the functionality should be marked as deprecated in a minor release > > and then removed as part of a major release. > > > > On Tue, Nov 28, 2017 at 7:03 PM, Jacob Barrett <jbarr...@pivotal.io> > > wrote: > > > > > Since the class was deprecated and is technically only there for > pre-1.0 > > > compatibly then the behavior of this class should be consistent with > the > > > pre-1.0 version. This will break 1.0 to 1.3 but anyone coding to a > > post-1.0 > > > version should not be using this deprecated class. > > > > > > > > > > On Nov 28, 2017, at 5:22 PM, Jason Huynh <jhu...@pivotal.io> wrote: > > > > > > > > *With your proposoal 1.0 - 1.3 users would have modify their source > > code > > > on > > > > the client and the server forthe function, correct?* > > > > > > > > If they start a new geode server 1.4+ and happened to extend > > > > functionAdapter (it was deprecated in 1.0) then they would have to > > > > recompile their client to not use functionAdapter. > > > > > > > > This should only affect users that extend FunctionAdapter and execute > > > > functions by serializing them to the server from the client. If they > > > > execute by id it should not run into this problem... > > > > > > > >> On Tue, Nov 28, 2017 at 5:20 PM Jason Huynh <jhu...@pivotal.io> > > wrote: > > > >> > > > >> Dan, yeah, the suggested change in the stack overflow answer does > work > > > and > > > >> I was able to put an if with the exact serialVersionUid before > posting > > > the > > > >> proposal, but it is pretty hacky and may affect another class that > > > somehow > > > >> generated the same uid. I can make that change too but I'd prefer > not > > > to > > > >> have to maintain it moving forward... > > > >> > > > >> > > > >> > > > >>> On Tue, Nov 28, 2017 at 5:09 PM Dan Smith <dsm...@pivotal.io> > wrote: > > > >>> > > > >>> I agree I don't think we can get rid of FunctionAdapter until the > > next > > > >>> major version. > > > >>> > > > >>> I was thinking FunctionAdapter is rather widely used, but then I'm > > > >>> surprised no one has hit this yet. > > > >>> > > > >>> All of the options kinda suck here - either pre 1.0 users have a > > > >>> compatibility issue or 1.0-1.3 users do. With your proposoal 1.0 - > > 1.3 > > > >>> users would have modify their source code on the client and the > > server > > > for > > > >>> the function, correct? > > > >>> > > > >>> If we got really fancy we could actually ignore the > serialVersionUUID > > > for > > > >>> this class like this - https://stackoverflow.com/a/1816711/2813144 > . > > > But > > > >>> it's pretty messy. > > > >>> > > > >>> -Dan > > > >>> > > > >>> On Tue, Nov 28, 2017 at 1:59 PM, Alexander Murmann < > > > amurm...@pivotal.io> > > > >>> wrote: > > > >>> > > > >>>> Anil, I am not sure following. I think FunctionAdapter already is > > > >>>> deprecated. Isn't it? Anthony is right though that we shouldn't > > remove > > > >>>> anything customer facing unless we are doing a major release. > > > Otherwise > > > >>> we > > > >>>> are violating the contract provided by semantic versioning. > > > >>>> > > > >>>> On Tue, Nov 28, 2017 at 1:52 PM, Anilkumar Gingade < > > > aging...@pivotal.io > > > >>>> > > > >>>> wrote: > > > >>>> > > > >>>>> I haven't seen many uses of FunctionAdapter; if its not used > much, > > I > > > >>>> think > > > >>>>> we should deprecate this... > > > >>>>> > > > >>>>> It only provided default implementation for few of the methods; > > this > > > >>>> could > > > >>>>> be added in the docs/release notes to help application to move to > > > >>>> function > > > >>>>> implementation. > > > >>>>> > > > >>>>> -Anil. > > > >>>>> > > > >>>>> > > > >>>>> On Tue, Nov 28, 2017 at 12:40 PM, Anthony Baker < > aba...@pivotal.io > > > > > > >>>> wrote: > > > >>>>> > > > >>>>>> I think we should wait for a major release to remove API’s. If > we > > > >>>> broke > > > >>>>> a > > > >>>>>> public API, we should fix that IMO. > > > >>>>>> > > > >>>>>> Anthony > > > >>>>>> > > > >>>>>> > > > >>>>>>> On Nov 28, 2017, at 11:40 AM, Patrick Rhomberg < > > > >>> prhomb...@pivotal.io > > > >>>>> > > > >>>>>> wrote: > > > >>>>>>> > > > >>>>>>> +1 to removing a long-deprecated class from the Geode side. > > > >>>>>>> > > > >>>>>>> On Tue, Nov 28, 2017 at 8:04 AM, Bruce Schuchardt < > > > >>>>>> bschucha...@pivotal.io> > > > >>>>>>> wrote: > > > >>>>>>> > > > >>>>>>>> How about just getting rid of this class? After all it was > > > >>> marked > > > >>>> as > > > >>>>>>>> being deprecated in 1.0. Pivotal could add a compatible > > > >>>>> FunctionAdapter > > > >>>>>>>> class in their GemFire builds to support these old clients. > > > >>>>>>>> > > > >>>>>>>> > > > >>>>>>>> > > > >>>>>>>>> On 11/27/17 10:18 AM, Jason Huynh wrote: > > > >>>>>>>>> > > > >>>>>>>>> This is a discussion for the fix to GEODE-4008: > > > >>>>>>>>> InvalidClassException when deserializing FunctionAdapter from > > > >>> pre > > > >>>>> Geode > > > >>>>>>>>> clients > > > >>>>>>>>> > > > >>>>>>>>> There was a change to deprecate FunctionAdapter in Geode > > (before > > > >>>>> 1.0), > > > >>>>>> and > > > >>>>>>>>> this also removed the method signatures in the class. This > > > >>> caused > > > >>>>> Java > > > >>>>>> to > > > >>>>>>>>> generate a new serialVersionUID to the class because one was > > not > > > >>>>>> assigned > > > >>>>>>>>> previously. However we have clients pre Geode that when they > > > >>>> attempt > > > >>>>> to > > > >>>>>>>>> execute a function by serializing the function across (not > > > >>> using a > > > >>>>>>>>> function > > > >>>>>>>>> id), the FunctionAdapter class is unable to deserialize > > > >>> properly. > > > >>>>>>>>> > > > >>>>>>>>> The proposed fix is to assign a serialVersionUID to the class > > > >>> that > > > >>>>>> matches > > > >>>>>>>>> that of the pre Geode FunctionAdapter. This will cause any > > Geode > > > >>>>>> 1.0-1.3 > > > >>>>>>>>> clients to now run into the error but the older clients would > > > >>> work > > > >>>>>> fine. > > > >>>>>>>>> Because FunctionAdapter has been deprecated it should be easy > > > >>>> enough > > > >>>>>> for > > > >>>>>>>>> Geode 1.0-1.3 users to change their custom classes to > implement > > > >>>>>> Function > > > >>>>>>>>> directly and not use the deprecated FunctionAdapter class. > > > >>>>>>>>> > > > >>>>>>>>> Please let me know if there is a better solution or if there > > are > > > >>>>>> problems > > > >>>>>>>>> with the proposed fix. > > > >>>>>>>>> > > > >>>>>>>>> > > > >>>>>>>>> Thanks, > > > >>>>>>>>> > > > >>>>>>>>> -Jason > > > >>>>>>>>> > > > >>>>>>>>> > > > >>>>>>>> > > > >>>>>> > > > >>>>>> > > > >>>>> > > > >>>> > > > >>> > > > >> > > > > > >
