[OPEN-ILS-DEV] ***SPAM*** Re: Deprecating APIs (was: osrfAppSessionMakeRequest(): eliminate param_strings?)

[Dan Scott]

On Fri, 2009-12-11 at 12:06 -0800, Scott McKellar wrote:
> 
> --- On Fri, 12/11/09, Dan Scott <dan at coffeecode.net> wrote:
> > Date: Friday, December 11, 2009, 9:11 AM
> > On Sat, 2009-12-05 at 09:09 -0800,
> > Scott McKellar wrote:
> > <snip>
> > > -----------
> > > 
> > > Migration path:
> > > 
> > > 1. Create a similar function with a different name and
> > no param_strings
> > > parameter.
> > > 
> > > 2. Replace all calls to the old function with calls to
> > the new one.
> > > 
> > > 3. Eliminate the old one.
> > > 
> > > 4. Change the underlying function
> > osrfAppSessionMakeLocaleRequest()
> > > along the same lines.  This will be easy because
> > it's not called from
> > > anywhere else.
> > > 
> > > 5. Eliminate osrf_message_add_param().
> > > 
> > > All that's different about
> > osrfAppSessionMakeLocaleRequest() is that it
> > > takes an additional parameter for locale, and in
> > practice that parameter
> > > is always NULL.  We could eliminate that
> > parameter as well, but since
> > > it might be useful some day, we might as well keep it
> > around.
> > 
> > I have no concerns about the technical approach for the
> > migration path.
> > 
> > In the future I suggest that we adopt a deprecation policy
> > where we mark
> > a given API as deprecated (via both the release notes - a
> > "deprecated
> > API" section- and an OpenSRF log at the warning level,
> > perhaps, with a
> > standard greppable format like "Deprecated API
> > <api_name(api_signature)>"?) and push that out in the
> > next release.
> > 
> > After which we reserve the right to remove the API in
> > future major
> > releases.
> 
> A lot depends on how you define "API".

Indeed.

> In the C world, one might arguably define the "API" as including
> everything that's declared in any header file.  However, things are
> exposed in our header files only as an artifact of the way the source
> code is physically packaged.  They aren't really intended or expected
> to be referenced by anything but our own code.
> 
> When I eliminate a function or change its signature, I can grep for all
> the places that call it, and modify them as needed in order to
> accommodate the change.  I've done that a LOT over the years.  If I had
> followed some kind of formal deprecation procedure, I would have spent a
> lot of time on that bureaucratic overhead, accomplished less substantive
> change, and left the code base littered with a lot of deprecated code
> that I didn't feel free to remove yet.  The chance that anyone would
> have benefited is probably close to zero.

Yes, I'm familiar with the refactoring process.

> OSRF as a product is not a collection of library routines with an API, like (for example) lib2xml.  It's primarily a collection of executables
> for message passing and remote procedure calls.  Normally the internal
> implementation is not publicly visible.

I strongly disagree. In the case of osrfAppSessionMakeRequest(), it is
used by the example C applications - which in my book makes it part of
the externalized API. If anyone else wants to create an OpenSRF service
written in C, they would also use this API. And they could have looked
at the signature, decided to use the param_strings argument, and gone on
their merry way until the next version of OpenSRF landed and broke their
application.

Even more convincing, Evergreen itself - which is nothing more than an
OpenSRF application and therefore must only use the external API -
calls osrfAppSessionMakeRequest().

> Since the source code is available to anyone who wants to download it,
> it is always possible that a change will adversely affect someone who
> is using the code in ways that we don't know about.  As far as I'm
> concerned, that's the risk you take when you monkey with the internals.

Yes, this is always possible. To avoid this problem, other projects and
products document or at least list the external API and warn that
writing to any of the internal functions is not supported and liable to
break with future upgrades, etc. Perhaps we should do the same so that
people will know that they're "monkeying" with the internals. Or more
importantly, so that there is a documented API that people can use to
actually build OpenSRF applications.

> A narrower definition of "API" would comprise:
> 
> 1. The structure and packaging of the messages passed over the wire;
> 
> 2. The way that Jabber and the router interact to link up clients and
> services;
> 
> 3. The protocols by which different processes talk to each other;
> 
> 4. The signatures of the methods provided by the various services;
> 
> 5. The calling conventions for dynamically loaded objects.
> 
> Those kinds of things are harder to change, and a formal deprecation
> process would make sense.  For internal details of implementation, not
> so much.

Except for the part about writing OpenSRF services in C. Again, to me
this function looks like part of the external API.

> It remains possible that people are using the internals more than I
> realize.  If so, perhaps we should somehow identify which internals
> deserve a formal deprecation policy and which don't.  I'd rather not
> be burdened by procedural overhead from which no one is likely to
> benefit.

All this said, even though this function is almost certainly a part of
the external API, I also doubt that making this particular change will
break anyone's usage, because let's face it - we don't have (m)any
people writing new OpenSRF services in C. Having a documented external
API might help change that, and it's certainly a prerequisite for
putting a deprecation process into place.