Make impl/memory compatible with appengine.APICallFunc hooks to allow existing codebases to use it.

[jongillham]

A few months ago a google.golang.org/appengine commit exposed functionality for intercepting API calls. It would be great if luci/gaes memory backend could plug into this for others to use who are just using the google.golang.org/appengine APIs.

The functionality would go something like this:

    import (
        "testing"

        "golang.org/x/net/context"
        "google.golang.org/appengine"

        "github.com/luci/gae/impl/memory"
    )

    func TestCode(t *testing.T) {
        ctx := context.Background()
        ctx = appengine.WithAPICallFunc(ctx, memory.APICallFunc)

        // User test code for example:
        if _, err := datastore.Put(ctx, key, entity); err != nil {
            t.Fatal(err)
        }
    }

I thought I would just put it out there – even though I understand it will probably never be on the roadmap. Too many open files errors are driving me crazy so I thought I would do something vaguely positive to try and get off dev_appserver.py for unit testing.

[riannucci]

Hi!

If I understand this correctly, this would allow impl/memory to intercept and process the SDK's protobuf commands that would normally have been sent to dev_appserver.py / real GAE?

If that's the case (and I didn't know they opened this API up, thanks for letting me know!), I think this is probably blocked on / related to #23 (essentially, get a handle on all of the protobufs and rewrite luci/gae's internals in terms of them).

But this would definitely be a worthwhile thing to do:exclamation: I understand the luci/gae api change vs. the GAE SDK is a pretty huge gap, and this would decouple the impl/memory backend implementation, which would be great because then existing codebases could seamlessly take advantage of the faster testing and such.

One thing I'm a bit worried about is that currently the SDK keeps all of its protobuf implementations in internal areas (e.g. https://github.com/golang/appengine/tree/master/internal/datastore) which will be inaccessible once things move to go 1.5 (I'm assuming that'll happen sometimeish). I think that would essentially mean that the api hook would need to encode + decode the protos on every call to get inputs, but I'm not sure how it would be able to generate outputs from that hook method, since the SDK code expects equivalent Go types. Maybe we could convince them to let their protos run free? 😄

[jongillham]

Glad you support the proposal!

That's a good point related to issue #23. How are we supposed to typecast the in and out parameters of appengine.APICall(ctx context.Context, service, method string, in, out proto.Message) error when the protobuf structs are internal?

I've asked on the google-appengine-go group to see if they have any suggestions.

[lorenzleutgeb]

FYI @dsymonds at some point stated that the protos will not be exposed. Not sure if he changed his mind since then.

[riannucci]

Hm... that would mean hiding the protos from all client implementations (java, python, go, php) though... that seems very unlikely to happen?

I kind-of assumed that with gRPC things would be moving in the more-protos direction, e.g. https://cloud.google.com/datastore/reference/rpc/google.datastore.v1beta3 .

That said, the initial implementation here would probably be 'fork them' (and then keep an eye on them to pick up any changes), as he suggests in that ML thread. AFAIK, they haven't changed substantially for a very long time (e.g. https://github.com/golang/appengine/commits/master/internal/datastore/datastore_v3.proto ). If this issue is resolved correctly, then all import "google.golang.org/appengine/*" would be removed from this repo anyhow, so whether they're importable via that repo or not wouldn't matter.

[riannucci]

Oh, I thought I was commenting on #23. If we do #23, then the resolution to THIS issue would probably be in the form of some in-process or out-of-process service that behaves like dev_appserver: existing codebases that do import "google.golang.org/appengine/*" would be able to connect to it as if it were dev_appserver.py for the purposes of unittesting. I expect we would also introduce some other APIs (probably via the same rpc channel, just new RPCs) which would allow you to create multiple 'fresh' app instances, so you could do one-per-unittest without having to spawn a new process per test, and also be able to control testing consistency features, e.g. datastore.Testable.

We already have a fair amount of bugfeature compatibility code in impl/memory; the fake dev_appserver would also continue to implement that stuff to provide an as-close-to-production experience as possible (e.g. limiting transaction sizes, number of modified entities in a single RPC, etc). At some point we should probably document all of these bug compatibility things with tiny demo apps. In fact, I'll file a bug for that right now: #46.

[dsymonds]

FYI, I doubt the protos in the form of the google.golang.org/appengine/internal/* packages will be made non-internal. They aren't part of the supported API surface there.

However, since those protos are used for communication between the app and the API server, they generally keep to the standard compatibility patterns that protos are known for (e.g. not changing tag numbers, etc.), so it would be pretty safe to have your own copy of the protos (and periodically sync) and use the APICall mechanism to do whatever you want. You'll need to do an extra proto.Marshal/proto.Unmarshal to bounce them between the real protos and your copy of the protos, but that should be fairly minimal overhead in the scheme of things.