should adding an API call require a new API version?

[John Meinel]

So I was hoping for a bit more developer participation, since I think we
need to decide as a group how we are going to handle this.

Here are some of my thoughts

   1. Just adding the API and having a "IsNotImplemented" check is
   certainly lower overhead.
   2. It does mean that from a version number you can no longer know a
   priori what APIs are going to be available. So if you ever do bump the
   version for another reason (and Client *really* needs a version bump
   badly), then you end up in a situation where you have to do a version
   check, and then do a NotImplemented check because the first doesn't
   actually cover everything.
   3. Adding an argument to an existing function clearly needs a version
   bump (because our silently drop policy means you can't know whether the
   other side actually accepted the new arguments). But if adding a new API
   doesn't then there is heavy bias towards just adding more API calls, rather
   than keeping a small clean set that does what we want. Which means you end
   up with
      1. AddCharm
      2. AddCharmWithCredentials
      3. AddCharmWithCredentialsAndConfig
      4. NoReallyThisIsTheAddCharmThatYouShouldBeUsing
      5. (A concrete example is NewServiceSetForClientAPI because we needed
      a new name and the implementor didn't come up with something simpler that
      just conveyed it interpreted the arguments in a different manner.)
   4. Juju-core itself will rarely pay consequences for any change we make,
   because we can always make sure 1.X can talk to 1.X. Where we run into
   problems is ensuring that 1.X can talk to 1.Y and making sure
   python-jujuclient can talk to us, etc. We've made the statement that we
   will maintain compatibility, but the group as a whole has not been very
   consistent here. We've ripped out code that meant 1.22 would be compatible
   with 1.18 (fallback code), and we've made changes to APIs, introduced new
   APIs without having them clearly marked as not-available-in-1.18. We also
   haven't been doing internal compatibility testing much.
      1. I'd *like* to see a case where when introducing API version 2, we
      test that version 1 *doesn't* expose the new functionality, and still
      behaves like version 1, as well as tests that version 2 acts correctly in
      the new fashion.
      2. I don't think we have to run all tests against all versions (for
      things that haven't changed), though if our tests are lightweight enough,
      then it isn't a big burden. Right now a lot of our API tests are
full-stack
      tests, so they aren't really that lightweight. Though given the amount of
      business logic in the API server layer, and that our model objects are
      directly tied to the DB, we don't have a huge amount of choice there.
   5. We've had a couple of pushes to shrink down the Client API (at the
   very least most new stuff gets created in a focused API instead of getting
   tacked onto Client.) What if we did that for AddCharm. We'd want to think
   about where it could sit, but a Charm api, or a Service api, or ...?
   Something that could collect a few APIs that are related instead of having
   all of Client. And then instead of just adding AddCharmWithCredentials to
   Client you can just create that new focused facade, and have one member on
   it for now. That is the same as bumping the version of Client but a lot
   cheaper and still gives us API versions that mean something. (The new
   Facade is added at version 1, so it clearly didn't exist in the 1.18 v0 of
   the API, and Client still has the same API at v0, etc.)

(5) may be the best solution to get something moving forward without having
to refactor all of Client, and it moves us further towards having small
focused Facades which are easier to version.

Thoughts?

John
=:->

On Thu, Apr 2, 2015 at 3:34 PM, roger peppe <roger.peppe at canonical.com>
wrote:

> To add a new API call there are at least two possible
> approaches.
>
> For example, say I want to add a new API call to the Client facade.
>
> 1) (the preferred approach as I understand it)
>
> - copy apiserver/client to apiserver/clientv1.
>
> - change the facade implementation in clientv1 so that
> it embeds the old client version, but keep all the
> tests intact.
>
> - call common.RegisterStandardFacade with the new version
> in clientv1.
>
> - implement the new call in clientv1 and add
> a test for that call.
>
> - implement the client side for the new call in api/client.go.
>
> 2) (the approach that most people have used so far)
>
> - implement the new call in clientv1 and add a test for that call.
>
> - implement the client side for the new call in api/client.go.
>
> I am wondering if the former approach is enough better than
> the latter to justify the costs that it imposes.
> The costs I'm thinking of are:
>
> - it grows the code significantly more and adds a significantly
> larger barrier to adding new API features.
>
> - the tests are now duplicated (not insignificant - the apiserver/client
> tests take >2minutes to run on my machine).
>
> - the resulting code is harder to understand as there's an extra
> layer of indirection on every API call to clientv1 (this will
> get worse as more versions are added).
>
> One advantage that I can see to the former approach is
> that it provides us with a tangibly different version number
> when someone tries to use a new client against an old server,
> which could result in a nicer error message in this case.
>
> On the other hand, as long as someone keeps track of
> which API calls are added in which juju version, an
> error message saying "API call MyNewCall not implemented"
> is probably just as useful, especially if it is printed along
> with a link to more information.
>
> Thoughts?
>
>   cheers,
>     rog.
>
> --
> Juju-dev mailing list
> Juju-dev at lists.ubuntu.com
> Modify settings or unsubscribe at:
> https://lists.ubuntu.com/mailman/listinfo/juju-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/juju-dev/attachments/20150407/ccbbeeef/attachment.html>